[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8ab4899c-ec12-a713-cac2-d951fff2a347@nvidia.com>
Date: Mon, 22 Jul 2019 23:33:32 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Christoph Hellwig <hch@....de>, <john.hubbard@...il.com>
CC: Andrew Morton <akpm@...ux-foundation.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Björn Töpel <bjorn.topel@...el.com>,
Boaz Harrosh <boaz@...xistor.com>,
Daniel Vetter <daniel@...ll.ch>,
Dan Williams <dan.j.williams@...el.com>,
Dave Chinner <david@...morbit.com>,
David Airlie <airlied@...ux.ie>,
"David S . Miller" <davem@...emloft.net>,
Ilya Dryomov <idryomov@...il.com>, Jan Kara <jack@...e.cz>,
Jason Gunthorpe <jgg@...pe.ca>, Jens Axboe <axboe@...nel.dk>,
Jérôme Glisse <jglisse@...hat.com>,
Johannes Thumshirn <jthumshirn@...e.de>,
Magnus Karlsson <magnus.karlsson@...el.com>,
Matthew Wilcox <willy@...radead.org>,
Miklos Szeredi <miklos@...redi.hu>,
Ming Lei <ming.lei@...hat.com>, Sage Weil <sage@...hat.com>,
Santosh Shilimkar <santosh.shilimkar@...cle.com>,
Yan Zheng <zyan@...hat.com>, <netdev@...r.kernel.org>,
<dri-devel@...ts.freedesktop.org>, <linux-mm@...ck.org>,
<linux-rdma@...r.kernel.org>, <bpf@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] mm/gup: introduce __put_user_pages()
On 7/22/19 10:53 PM, Christoph Hellwig wrote:
> On Mon, Jul 22, 2019 at 03:34:13PM -0700, john.hubbard@...il.com wrote:
>> +enum pup_flags_t {
>> + PUP_FLAGS_CLEAN = 0,
>> + PUP_FLAGS_DIRTY = 1,
>> + PUP_FLAGS_LOCK = 2,
>> + PUP_FLAGS_DIRTY_LOCK = 3,
>> +};
>
> Well, the enum defeats the ease of just being able to pass a boolean
> expression to the function, which would simplify a lot of the caller,
> so if we need to support the !locked version I'd rather see that as
> a separate helper.
>
> But do we actually have callers where not using the _lock version is
> not a bug? set_page_dirty makes sense in the context of a file systems
> that have a reference to the inode the page hangs off, but that is
> (almost?) never the case for get_user_pages.
>
I'm seeing about 18 places where set_page_dirty() is used, in the call site
conversions so far, and about 20 places where set_page_dirty_lock() is
used. So without knowing how many of the former (if any) represent bugs,
you can see why the proposal here supports both DIRTY and DIRTY_LOCK.
Anyway, yes, I could change it, based on your estimation that most of the
set_page_dirty() calls really should be set_page_dirty_lock().
In that case, we would end up with approximately the following:
/* Here, "dirty" really means, "call set_page_dirty_lock()": */
void __put_user_pages(struct page **pages, unsigned long npages,
bool dirty);
/* Here, "dirty" really means, "call set_page_dirty()": */
void __put_user_pages_unlocked(struct page **pages, unsigned long npages,
bool dirty);
?
thanks,
--
John Hubbard
NVIDIA
Powered by blists - more mailing lists