From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753288AbcK0CZ2 (ORCPT ); Sat, 26 Nov 2016 21:25:28 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:41630 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752872AbcK0CZV (ORCPT ); Sat, 26 Nov 2016 21:25:21 -0500 Date: Sun, 27 Nov 2016 02:25:09 +0000 From: Al Viro To: Linus Torvalds Cc: Linux Kernel Mailing List , linux-fsdevel Subject: Re: [git pull] vfs fix Message-ID: <20161127022506.GW1555@ZenIV.linux.org.uk> References: <20161127011352.GV1555@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Nov 26, 2016 at 05:48:54PM -0800, Linus Torvalds wrote: > That's what all the other users do, and that's what should be the > "right usage pattern", afaik. The number of pages really *is* > calculated as > > int n = DIV_ROUND_UP(result + offs, PAGE_SIZE); > > in other iov_iter_get_pages_alloc() callers, although tghe nfs code > open-codes it as > > npages = (result + pgbase + PAGE_SIZE - 1) / PAGE_SIZE; > > so it's not a very strong pattern. Two issues here. One is that iov_iter_get_pages{,_alloc}() calling conventions are fucking ugly. I'm guilty of that atrocity; my only excuse is that this thing has congealed from many open-coded instances, quite a few of those appearing only after considerable massage of the code. I _hate_ the boilerplate we have in the functions implementing those for various iov_iter flavours and boilerplate in the callers. I am going to try and come up with something less atrocious. As it is, renaming that variable and adding it to the return value of iov_iter_get_pages_alloc() is certainly not a problem and would be prettier, but TBH I just went "yet another place to go into that cleanup". Shouldn't have. Another thing is that it was a leftover from "Alexei, could you see if that thing fixes your reproducer?" - just in case the things _really_ went insane and it was not a wrong rounding but somehow completely buggered pipe_get_pages_alloc(). They hadn't. Anyway, leaving that BUG_ON() had been wrong; I can send a followup massaging that thing as you've suggested, if you are interested in that. But keep in mind that the whole iov_iter_get_pages...() calling conventions are going to be changed, hopefully soon.