Re: [PATCH] iomap: support partial page discard on writeback block mapping failure

[Brian Foster <bfoster@redhat.com>]

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