On Wed, Oct 28, 2020 at 07:31:27AM +0000, Christoph Hellwig wrote:
> > if (unlikely(error)) {
> > + unsigned int pageoff = offset_in_page(file_offset);
> > + /*
> > + * Let the filesystem know what portion of the current page
> > + * failed to map. If the page wasn't been added to ioend, it
> > + * won't be affected by I/O completion and we must unlock it
> > + * now.
> > + */
> > + if (wpc->ops->discard_page)
> > + wpc->ops->discard_page(page, pageoff);
>
> I don't think we need the pageoff variable here. Also it would
> seem more natural to pass the full file_offset offset instead of
> having to recreate it in the file system.
>
I used the variable just to avoid having to split the function call into
multiple lines. I.e., it just looked more readable to me than:
if (wpc->ops->discard_page)
wpc->ops->discard_page(page,
offset_in_page(file_offset));
I can change it back if that is preferred (or possibly use a function
pointer variable instead). I suppose that's also avoided by passing
file_offset directly, but that seems a little odd to me for a page
oriented callback.
Brian
