Re: [PATCH] fs: kill unused ret2 argument from iocb->ki_complete() - Greg Kroah-Hartman

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: John Keeping <john@metanate.com>,
	linux-usb@vger.kernel.org, Christoph Hellwig <hch@infradead.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH] fs: kill unused ret2 argument from iocb->ki_complete()
Date: Sat, 23 Oct 2021 11:09:21 +0200	[thread overview]
Message-ID: <YXPRQV1BT2yYYOgN@kroah.com> (raw)
In-Reply-To: <e39e7f45-1c1e-a9bb-b413-1dfc21b1b20f@kernel.dk>

On Fri, Oct 22, 2021 at 09:44:32AM -0600, Jens Axboe wrote:
> On 10/21/21 10:40 AM, John Keeping wrote:
> > On Wed, 20 Oct 2021 19:49:07 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> >> On Wed, Oct 20, 2021 at 11:35:27AM -0600, Jens Axboe wrote:
> >>> On 10/20/21 11:30 AM, Christoph Hellwig wrote:  
> >>>> On Wed, Oct 20, 2021 at 10:49:07AM -0600, Jens Axboe wrote:  
> >>>>> It's not used for anything, and we're wasting time passing in zeroes
> >>>>> where we could just ignore it instead. Update all ki_complete users in
> >>>>> the kernel to drop that last argument.
> >>>>>
> >>>>> The exception is the USB gadget code, which passes in non-zero. But
> >>>>> since nobody every looks at ret2, it's still pointless.  
> >>>>
> >>>> Yes, the USB gadget passes non-zero, and aio passes that on to
> >>>> userspace.  So this is an ABI change.  Does it actually matter?
> >>>> I don't know, but you could CC the relevant maintainers and list
> >>>> to try to figure that out.  
> >>>
> >>> True, guess it does go out to userspace. Greg, is anyone using
> >>> it on the userspace side?  
> >>
> >> I really do not know (adding linux-usb@vger)  My interactions with the
> >> gadget code have not been through the aio api, thankfully :)
> >>
> >> Odds are it's fine, I think that something had to be passed in there so
> >> that was chosen?  If the aio code didn't do anything with it, I can't
> >> see where the gadget code gets it back at anywhere, but I might be
> >> looking in the wrong place.
> >>
> >> Anyone else here know?
> > 
> > I really doubt anyone uses io_event::res2 with FunctionFS gadgets.  The
> > examples in tools/usb/ffs-aio-example/ either check just "res" or ignore
> > the status completely.
> > 
> > The only other program I can find using aio FunctionFS is adbd which
> > also checks res and ignores res2 [1].  Other examples I know of just use
> > synchronous I/O.
> 
> So is there consensus on the USB side that we can just fill res2 with
> zero? The single cases that does just do res == res2 puts the error
> in res anyway, which is what you'd expect.
> 
> If so, then I do think that'd be cleaner than packing two values into
> a u64.

I think yes, we should try that, and if something breaks, be ready to
provide a fix for it.

thanks,

greg k-h