Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1693908

Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite()

Show all headers | View raw

On Thu, Jul 20, 2017 at 09:59:22AM -0600, Ross Zwisler wrote:
> On Thu, Jul 20, 2017 at 11:26:16AM -0400, Vivek Goyal wrote:
<>
> > Hi Ross,
> > 
> > vm_insert_mixed_mkwrite() is same as vm_insert_mixed() except this sets
> > write parameter to inser_pfn() true. Will it make sense to just add
> > mkwrite parameter to vm_insert_mixed() and not add a new helper function.
> > (like insert_pfn()).
> > 
> > Vivek
> 
> Yep, this is how my initial implementation worked:
> 
> https://lkml.org/lkml/2017/6/7/907
> 
> vm_insert_mixed_mkwrite() was the new version that took an extra parameter,
> and vm_insert_mixed() stuck around as a wrapper that supplied a default value
> for the new parameter, so existing call sites didn't need to change and didn't
> need to worry about the new parameter, but so that we didn't duplicate any
> code.
> 
> I changed this to the way that it currently works based on Dan's feedback in
> that same mail thread.

Looking at this again, I agree that duplicating vm_insert_mixed() seems
undesirable.  For v4 I'll add the flag to vm_insert_mixed() and just update
all the call sites instead of adding a separate wrapper for the mkwrite case,
which will fix this duplication and address Dan's naming concerns.

Thanks for the review feedback.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Vivek Goyal <vgoyal@redhat.com> - 2017-07-20 17:30 +0200
  Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-07-20 18:00 +0200
    Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-07-21 20:10 +0200

csiph-web