From patchwork Fri Sep 23 14:11:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yury Norov X-Patchwork-Id: 15954 Received: (qmail 11473 invoked by alias); 23 Sep 2016 14:12:23 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 11447 invoked by uid 89); 23 Sep 2016 14:12:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=n32 X-HELO: NAM02-CY1-obe.outbound.protection.outlook.com Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Yuri.Norov@caviumnetworks.com; Date: Fri, 23 Sep 2016 17:11:54 +0300 From: Yury Norov To: Adhemerval Zanella CC: Florian Weimer , Chris Metcalf , Subject: Re: [PATCH] Consolidate Linux readahead() implementations Message-ID: <20160923141154.GA970@yury-N73SV> References: <1474577068-1781-1-git-send-email-ynorov@caviumnetworks.com> <827f758c-b744-22f2-5dbb-4471208cd6b2@linaro.org> <20160922232119.GA12837@yury-N73SV> <87k2e3jgh8.fsf@mid.deneb.enyo.de> <20160923124456.GA22674@yury-N73SV> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-ClientProxiedBy: DB5PR10CA0038.EURPRD10.PROD.OUTLOOK.COM (10.165.4.176) To SN1PR07MB2256.namprd07.prod.outlook.com (10.164.47.150) X-MS-Office365-Filtering-Correlation-Id: 802f786e-259d-4b3d-75c6-08d3e3bb9988 X-Microsoft-Exchange-Diagnostics: 1; SN1PR07MB2256; 2:x/xMNqn2/JGb2NZyy/JG7PlJOLVDYdbfh4YIVXWqA9UbARK70dvqJIsnwb6ZwOFkXweiF5e9Z60aJSu89YtIlKe8dmA3FLs5NGf3BvJHOOMSX6NIg/7Vg+QKUuFScFHdLFXGaBer1Cv2SGLf3d4Yqhk3UczhFxkUgzdYAsiJxarm1iBM1e9oFsMmj/E+CkXP; 3:8/1XLMioK1gWnr91i78Fetu3LzA8eQeEPHflIYV/Ho3A6C8u2aO4a6hgEV7L0OqlrTMZ+NTuWQKmVHBq3t9ZgzHwhqP8++pjxTPo21/y73zZ+JEk4z49n7aoata56Q6Q X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR07MB2256; X-Microsoft-Exchange-Diagnostics: 1; SN1PR07MB2256; 25:D+BSUmKTaAeQcfqbFBnfjukKJ8dJH8/RXHRItYa3cOMQup+1jqtNDyAYb5xZQhecpBh6BBZl8YfyM+md9DJL3B7ZPA4xsBaXhV4jzrYTo1i+3bGs4Nkrcxk6J2WHGx+hCntmRMhnXuDJsB9M3e06Tf8cqgT95DMznAkShL1Nqdti3/MjVEs4xrM8x1CfENW6GUvXYAP1ybsQbKwjOxQwxdQOl6FbfI4zm95wURhpfJPxTmmqrIrgcjvzGabKWvECPioiSvuMNS+yA0lAscBRG84pYQLMrTkEafPmn2UgCitsqMvJ1nS56DzaH6nZeTpLRlbR6lY17c4xzOFiX9bKeER8+P0tpmGvqGDuJIIjN4qvsjIaRVPtPaRYt38Rq/tqfFLSvHzjmNb/ed3qfp0BZt0bvSkwql7fmGaKRRfjTghrhHyIUebU37ACsy47CUVKrH7OH5xAqjyc0lL/uwGbIZHbqoQshXjPIqJ9W+/Q9wguEDDfZMaazR3azoDwruUhb8hqlVo0ZbCePCm639tFAkOBW+TTaGlgXgugPrsprbD8WWFc2Vv2E0sNgISBDFIDp587Q6myK59mnX4ObthNyVteFlS2CcYlKoeYfYZYTo5RfaPpYmZAPBiy1zKBtSTcZ9xcA75m9m6iqbT6Trr6T+LM8aES86BnRlEhT5dfAmvywlHjRYGF5YyF+/2amtXq2nMCOIg03lpzsrZqA1pffxPzgwknmQ8LpNiUkQnM8LoRn/rlxvn8lyWyhuTI7uU3yl5R8Tvbq5R2PbN00Dhedx6myo3hX1qebeGFjUupIlJ7j+tsjDHsqbLawFFFLHBBIOqKhftYEvikZjTPBJ4PAt0lmbVW/BXPYjwjvQwYuuTUd0Ms/v1lrPUTISAmrWr/ X-Microsoft-Exchange-Diagnostics: 1; SN1PR07MB2256; 31:UJTuq2qdW902Sb6D8X0Ta4X5Te4wWBshUrBatMQnPZkzUrcasXdK0IJaEbSgY2VIrezd4wVNsldV78lPheTdlLx55R92UnledibLuwak3Lq4dyQOOGBG3jM267GNLpxLBDI8ce/yyq2DQPvQs8N1XUbW6xU77hDk4WdLEsL6AwridLcviuFG4faUblrdcZAAa1FjtZqRykwRBEE2tL4v3l9Z7J92UC3OP4Ud1eqo+jw=; 20:4SZkRu/bxFUvJBb0JExgcldDpH4tfiPhIHhVaHC1+vKaTNIGfknqBjNBBk3+PfjtBdLwnjJ/FgSYrHMYq3IoI1OyjqJ1231gKsRSOdogM706rQy58r5W1TsUhPl4WkfcJza690yblevBTUxRq44x6PmNDsv5M3EGhYCZauS/lms/65vH/2TPvvxwbrNxezWINtUMtSakH0A30BQmuUUOzpbZYpAuowi307jCr4Tx8HCjdjfcI0WC0bxzvG/qI3ydV46/RZ5ZPj3tf8PLVbrVN8uIfEbEkD++eePK+D7Nbssjh7GjjSOx1TRdSCzi9w2zwIfLBhn7+kRA6XQ0Zq6Ev4JjDcgqF/LnFHE+ltdNYpgi1BDW/AyHXmvG8YsdHW7qpu65sHZFrMHL/PNCsCIcOaUL0JTdoEMbjFgnfJrTv9xtCEOKul83dwp2w5zx/gTUnZWv4TIqKHuR9tx5YQJfXfo290lz06rns80SzT5aYcAyHKGSbIAbiA4EO3FFpSQVOu1MC8WkbQxDfJGsXXYIcDW8Wy+eaatWQqaEY4XzKDAHVXayLYyJki4ETH1LllmobcKSIxzfOcsfyLARpO8oBDwC+e7F//u4F2FbuvzSLiU= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046); SRVR:SN1PR07MB2256; BCL:0; PCL:0; RULEID:; SRVR:SN1PR07MB2256; X-Microsoft-Exchange-Diagnostics: 1; SN1PR07MB2256; 4:MvZjovO50EcZwHaCiI/J3oxgbTezw4F5bkBnT10hIacceK+cu+QwsobLGxB3j7MVVOnOPebY0tLeBpP1yxryEWyqYg/cfDG1XmoTtzBeWrGGQxQAzos3/CVvDYDnoUzHyQkHJMMsodIWxHU5z1MJHKIxlkbAaV7spafwVo/2iTAvu4LdKusj5kQB9Ux4l7wbrR+9Guw9BASRHW1damMQ3gfPcYL9BeYOfJsQ2mkmWGIU1eUDofO84iiwn32fviMdsysvGzc1kGfnFaudP0KgPAOqE5UPc5ronv6ri5B2GY8Q07uwD14Q8b8gu7yGfpP9MboAUNlDGB6byp6Mcqy6x6FWAR4v8OS8kTRwyMtCn2gBVqpmyqMcZLcPdkelTVyHsvFX61tZTLOocES7diGqIw== X-Forefront-PRVS: 0074BBE012 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6069001)(6009001)(7916002)(189002)(199003)(24454002)(33716001)(105586002)(9686002)(76506005)(42186005)(66066001)(19580395003)(50466002)(83506001)(586003)(81156014)(8676002)(81166006)(23726003)(7846002)(305945005)(7736002)(101416001)(110136003)(33656002)(4001350100001)(575784001)(97736004)(2950100001)(97756001)(4326007)(92566002)(68736007)(106356001)(1076002)(2906002)(3846002)(6116002)(47776003)(46406003)(54356999)(50986999)(76176999)(77096005)(5660300001)(15975445007)(93886004)(189998001)(6916009)(18370500001); DIR:OUT; SFP:1101; SCL:1; SRVR:SN1PR07MB2256; H:localhost; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: caviumnetworks.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; SN1PR07MB2256; 23:oQaQ7UdvENZOutlwBF/SDtGG4rZXxkA07dT8Q3+Tq?= =?us-ascii?Q?t4rNJOf6PHH8XDrFJnB4d2l2vXlzEMY30TAmcLiAtCfMqYv8aFBrp/OxZB0v?= =?us-ascii?Q?3ulJmzMPIgtYD3HWcPr9JdL5MCMLncb52cKOLHYcyhOFXndAdGV6ObmoZJDh?= =?us-ascii?Q?FHatrqeaDKyiIYNMrYn+7e3Tz/dZvneuJuCyZ9vHfnUgyFIioBWouyUk88bU?= =?us-ascii?Q?Fjx1qlIosQICojizGhQxq1uDVM971NT02bYIw+pylZdoxFk2h7DkGPQY9Xbc?= =?us-ascii?Q?hk7LEi9kdtbNex4L1d9jfVFGVEW6fYDIfyCHUDC2FLn/b5QVZ9ImdNwl+AuE?= =?us-ascii?Q?x1abzmHf38gx0PGjyW3+i62ZdWKpGMdDYABZA9PYmgSkrDr/dCvv9LcW89HI?= =?us-ascii?Q?ZTALCJZjInkv3/8DhQsJy66IBbB5V8fkNwj50o9GRc0rrC2ngiYP77yVaf0J?= =?us-ascii?Q?hooShfPIU8YiyajdBFQL0wixFLARmGTN+dW419pfvywkHPR+iDAUINYCxz/r?= =?us-ascii?Q?A15YJQJguJZjz+uxA86Sc9wlZP1WvcQNDyF+NhF6prcJ3WnmbZp+hnfQgDMA?= =?us-ascii?Q?gWGDU+uVbH9Qnsz1ZnWHput9GGUU6xqE5O5ncU+zFBjqXqtJgv9Ejr5qymnZ?= =?us-ascii?Q?nGVQeUvpSdxeidMXgzUWD0iCm5HG16BtwgbPbHZSsqq4zx2HiDSfxgYjT+DH?= =?us-ascii?Q?QAgixuSeL98X77Ilo3TgsoGoIzfhZD721Dl5ZJ9m7EMWmgTEGhTXfKAIfpWW?= =?us-ascii?Q?iuPT3EWkDuPGZIQwTxuRLAPZrTvQ0ue/jyhgMvWMICtul4w25kUpaWAeQnSe?= =?us-ascii?Q?GQM2LK5hoGZ5BTHoXj3EPg6Tv5QPQ14ICTACh2+YwNSqGxhloKYbAJznokzX?= =?us-ascii?Q?e/2GnnmpncfJpX/3FxQ6Qpnn9bsK7BcYT34fl+MSiIzZoazURt+mdwu1E0GE?= =?us-ascii?Q?0hoBRQ/PqRtV3kNYP8iKgwA6iWmxAvj6ziqknLIxrC5Xp8taLWTF2P74ZbeJ?= =?us-ascii?Q?RIkm+guozvTRwiM/DoXgFFUjy6uW+v4Ei2/4ona0CCHG51+zSdf8C2u4+/8n?= =?us-ascii?Q?Q2eUzlyoVh/FM6lDI41beGnQwcb9pj+EfoFvPodieuVhZ6AirLoiK0fBkOpF?= =?us-ascii?Q?DbcksGHgdoEY08z3e6HhUYNF13ohNqV+D6lkmQErqmVeiUGjV9+J6Jh8AXcN?= =?us-ascii?Q?w3DYnV0RckFJ+LzWeZN/qiID/YhyDt1Rn8oL4hURsJk8unJEgSoS+oELorCd?= =?us-ascii?Q?r57rIgsRgofST1uX9eGyn/fsPtPYQToaPemUY0wjhDCjdPv2O6JqPp99m2Rs?= =?us-ascii?B?UT09?= X-Microsoft-Exchange-Diagnostics: 1; SN1PR07MB2256; 6:7TNkFQzyRjTfPelJAU1LcZQVA0PNtX18e4hc8H7JDo+ZuBXxjq633IhE7k2IdxaGabFUDqmrXP7z5USVUg1QL2KQy++/hpoJx6uvEwdkM0K1WNj95DTsM3Yp02YLZGCv5FQqSgG0nRdUoYP8CCZ+OMw8mCUqHCkTp0agmKFywOpZ/WMFU4cXemSRaNyfElkhfyzI55CAwRDSdlUt07kuw6Mi2JT7F9ZNZZsABC+iEybgQ3O/xh1X0+pMbfC4xlFNd814cr7J2FqMZ5kZKojny/M+z9JFC/xmfGakZFfoYe4=; 5:L4d5EVe1QHKejhZBOYEcGUG1V/XDm7b7q3oRxbmT0XglDByBIwLzVSlQX7TU2gyyxEPxwvXQ86Lg2UeeiCwezSWOG/TNFo5KBqwnKtykktcdo4upmhzLHTVIvMfvB2FzSl1xfwqBJv5LHpk3CV5Syw==; 24:/Skj0ZK6R9R6XB6yDJn+kopNN8hrHQEJbZ35V9oaOuh2lQ10YrVdXxSmAVm+Tn1D4n+NjF0AeJGFq4cH/Gg2pdeqBb4hbdh3kRNLKSII+Is=; 7:Dv6EjOiSTCqMMwNu0QhuSn/kwxReTLsuqUyIrKhDOocvac+o5kCL6cyoYggN1JGQ1TF6ECfHfJF5Sw5uwGxH8Gb6stj7vRaHeOsUMw5twenzt1jWEgxIlxnjyFJU56CjBgoRkxIGXsByFA56YsseFO0vchBSYXV+fixB4KLqJAEM5r4maf6HR0C3WTKQgE8pXV82T818ft63X7UJq9hXJopCqBxc3nNBb7T0V3jOZg0bWOmJxgbX62mBpcSxyENSwpvVo6ql+T1Ue1sAsgK/41h9ZG2+2KGotpKA/fDMylHVcERrQZJjm5JeU8M90Rin SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Sep 2016 14:12:05.9506 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR07MB2256 On Fri, Sep 23, 2016 at 10:32:35AM -0300, Adhemerval Zanella wrote: > > > On 23/09/2016 09:44, Yury Norov wrote: > > On Fri, Sep 23, 2016 at 08:08:19AM +0200, Florian Weimer wrote: > >> * Yury Norov: > >> > >>> On Thu, Sep 22, 2016 at 06:36:22PM -0300, Adhemerval Zanella wrote: > >>>> Hi Yury, some comments below. > >>>> > >>>> On 22/09/2016 17:44, Yury Norov wrote: > >>>> I think we can use SYSCALL_LL64 plus __ALIGNMENT_ARG here. The tricky is to pass > >>>> the correct argument size, since it used by the macro to select the correct > >>>> arch-dependent one. This is exactly why I proposed the patch to add > >>>> {INTERNAL,INLINE}_SYSCALL_CALL [1], readahead will be simplified to just: > >>>> > >>>> ssize_t > >>>> __readahead (int fd, off64_t offset, size_t count) > >>>> { > >>>> return INLINE_SYSCALL_CALL (readahead, fd, > >>>> __ALIGNMENT_ARG SYSCALL_LL64 (offset)); > >>>> } > >>>> > >>>> I suggest you to wait the {INTERNAL,INLINE}_SYSCALL_CALL patch to land and > >>>> based this one on it. I think I addressed most of Florian comments, I just > >>>> need to check if assembly generated using the new macros is the same as > >>>> before (which I expect to). > >>>> > >>>> [1] https://sourceware.org/ml/libc-alpha/2016-09/msg00452.html > >>> > >>> __ALIGNMENT_ARG is controlled by __ASSUME_ALIGNED_REGISTER_PAIRS, > >>> as well as __ALIGNMENT_COUNT(). Currently we have 4 ports that > >>> enable it: mips, arm, powerpc and tile. Mips and arm pass 5 arguments, > >>> so __ASSUME_ALIGNED_REGISTER_PAIRS is what they need. Powerpc uses > >>> syscalls.list (thanks for pointing it), and so is not affected by this > >>> change. But tile is still in case. Is my understanding correct that it > >>> uses linux/readahead.c as implementation? If so, this patch will break > >>> tile ABI. That's why I introduced new option. > >>> > >>> So, as for me we cannot use __ASSUME_ALIGNED_REGISTER_PAIRS because of > >>> tile. Is it correct? > >> > >> Does readahead even work on tile today? Maybe it's already broken and > >> this change fixes it. :) > >> > >> Chris? > > > > In kernel 32-bit tile expects 4 arguments: > > > > arch/tile/kernel/sys.c: > > 61 #if !defined(__tilegx__) || defined(CONFIG_COMPAT) > > 62 > > 63 #ifdef __BIG_ENDIAN > > 64 #define SYSCALL_PAIR(name) u32 name ## _hi, u32 name ## _lo > > 65 #else > > 66 #define SYSCALL_PAIR(name) u32 name ## _lo, u32 name ## _hi > > 67 #endif > > 68 > > 69 ssize_t sys32_readahead(int fd, SYSCALL_PAIR(offset), u32 count) > > 70 { > > 71 return sys_readahead(fd, ((loff_t)offset_hi << 32) | offset_lo, count); > > 72 } > > 73 > > 74 int sys32_fadvise64_64(int fd, SYSCALL_PAIR(offset), > > 75 SYSCALL_PAIR(len), int advice) > > 76 { > > 77 return sys_fadvise64_64(fd, ((loff_t)offset_hi << 32) | offset_lo, > > 78 ((loff_t)len_hi << 32) | len_lo, advice); > > 79 } > > 80 > > 81 #endif /* 32-bit syscall wrappers */ > > > > So it seems we have no chance to consolidate getdents() completely w/o > > new option. If no objections, I'll send v2 with removing getdents from > > powerpc syscalls.list, and rework new option in more suitable way. > > Objections? > > > > Yury. > > > > Indeed, unfortunately tile seems to get its own readahead definition. > However I think it should not prevent us to use my previous strategy, > we can follow the SH example for pread (where it adds a dummy argument > before offset), and do something as: > > sysdeps/unix/sysv/linux/tile/readahead.c > > #include > > #ifndef _LP64 > /* Although tile 32-bit ABI passed 64-bit arguments in even registers, > readahead interface does not follow this convention. */ > # undef __ALIGNMENT_ARG > #endif > > #include Currently it looks like this to me (see below). If you think that separated file is better than new option - I'm OK with it, but I think it's strange because in other patches of series you introduce options (if I'm not mistake). We also have 2 another implementations - in linux/wordsize-64/syscalls.list and linux/mips/mips64/n32/syscalls.list. I think wordsize-64 is safe to generalize, but I'm worry about mips64. If we'll choose adding new options and so having a single file, it seems, we'll have to add another option for mips64/n32, like this: #if __ASSUME_READAHEAD_NO_PAIRS # define SYSCALL_LL64(val) (val) #endif If we choose 3 implementations, we can introduce no option, but have generic, tile and mips separated versions. Yury. diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h index 71ce57a..ba7d745 100644 --- a/sysdeps/unix/sysv/linux/kernel-features.h +++ b/sysdeps/unix/sysv/linux/kernel-features.h @@ -50,6 +50,11 @@ #define __ASSUME_ST_INO_64_BIT 1 #endif +#ifndef __ASSUME_READAHEAD_ALIGN +/* readahead() adds padding to registers if this control is enabled. */ +# define __ASSUME_READAHEAD_ALIGN 1 +#endif + /* The statfs64 syscalls are available in 2.5.74 (but not for alpha). */ #define __ASSUME_STATFS64 1 diff --git a/sysdeps/unix/sysv/linux/readahead.c b/sysdeps/unix/sysv/linux/readahead.c index 92e5428..eea3142 100644 --- a/sysdeps/unix/sysv/linux/readahead.c +++ b/sysdeps/unix/sysv/linux/readahead.c @@ -23,16 +23,20 @@ #include #include +#include #ifdef __NR_readahead +#if !__ASSUME_READAHEAD_ALIGN +# undef __ALIGNMENT_ARG +# define __ALIGNMENT_ARG +#endif + ssize_t __readahead (int fd, off64_t offset, size_t count) { - return INLINE_SYSCALL (readahead, 4, fd, - __LONG_LONG_PAIR ((off_t) (offset >> 32), - (off_t) (offset & 0xffffffff)), - count); + return INLINE_SYSCALL_CALL (readahead, fd, __ALIGNMENT_ARG + SYSCALL_LL64 (offset)); } #else ssize_t diff --git a/sysdeps/unix/sysv/linux/tile/kernel-features.h b/sysdeps/unix/sysv/linux/tile/kernel-features.h index ded0e43..15ad2d3 100644 --- a/sysdeps/unix/sysv/linux/tile/kernel-features.h +++ b/sysdeps/unix/sysv/linux/tile/kernel-features.h @@ -24,4 +24,5 @@ #ifndef _LP64 # define __ASSUME_ALIGNED_REGISTER_PAIRS 1 # define __ASSUME_FADVISE64_64_NO_ALIGN 1 +# define __ASSUME_READAHEAD_ALIGN 0 #endif