[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH libnbd v3 2/3] copy: Preserve the host page cache when reading from local files.



On Thu, Feb 25, 2021 at 03:25:48PM -0600, Eric Blake wrote:
> On 2/25/21 11:34 AM, Richard W.M. Jones wrote:
> > +/* If we are going to attempt page cache mapping which tries not to
> > + * disturb the page cache when reading a file.  Only do this on Linux
> 
> Works, but took me a couple reads to decide it wasn't a sentence
> fragment.  Would it be any better as s/If/Whether/?

I'll see if I can clarify this before I push it.

> > +static void page_size_init (void) __attribute__((constructor));
> > +static void
> > +page_size_init (void)
> > +{
> > +  page_size = sysconf (_SC_PAGE_SIZE);
> 
> Technically, sysconf() can fail; but if it fails on _SC_PAGE_SIZE,
> you've got bigger problems ;)

Maybe best to assert if it fails?

> > +  /* Only bother with whole pages. */
> > +  offset = ROUND_UP (orig_offset, page_size);
> > +  len = orig_len - (offset - orig_offset);
> > +  len = ROUND_DOWN (len, page_size);
> 
> What assurance do we have that the rest of nbdcopy is favoring a stride
> that is a multiple of page_size, so that we aren't stranding partial
> pages everywhere?

It's not an assurance, but in general nbdcopy will favour blocks
aligned to very large boundaries (32M).  From the level of
copy/file-ops.c we can't really be sure of this and so must assume
arbitrary alignment.

> Looks good.

Thanks for the reviews.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]