lkml.org 
[lkml]   [2019]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] mm/gup: introduce __put_user_pages()
From
Date
On 7/22/19 10:53 PM, Christoph Hellwig wrote:
> On Mon, Jul 22, 2019 at 03:34:13PM -0700, john.hubbard@gmail.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

\
 
 \ /
  Last update: 2019-07-23 08:36    [W:0.055 / U:4.988 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site