Re: [PATCH 1/2] block: make __register_blkdev() return an error

[Luis Chamberlain <mcgrof@kernel.org>]

On Tue, Sep 07, 2021 at 08:28:06AM -0700, Luis Chamberlain wrote:
> On Wed, Sep 08, 2021 at 12:23:02AM +0900, Tetsuo Handa wrote:
> > On 2021/09/07 23:57, Luis Chamberlain wrote:
> > >> Actually, blk_request_module() failures should be ignored, for
> > >> subsequent ilookup() will fail if blk_request_module() failed to
> > >> create the requested block device.
> > > 
> > > Then how about this:
> > > 
> > > Since we would like to use __must_check for add_disk() we proceed with
> > > the change to capture the errors and propagate them and we just document on
> > > fs/block_dev.c's use of blk_request_module() about the above issue and
> > > how we prefer the errror that ilookup() would return.
> > 
> > Marking add_disk() as __must_check makes it possible to enforce "don't leave
> > partially initialized devices". That's already an enough improvement.
> > 
> > Probe functions can remain "void", and hence blk_request_module() can remain "void".
> > That is, I would drop "[PATCH 1/2] block: make __register_blkdev() return an error".
> 
> Probe calls can be left voide, but because of the new __must_check we'd
> still have to modify all probe calls as they use add_disk() and it would
> seem odd to just capture the error to ignore it without documenting
> any of this.

So indeed, all probe() routines would need to be modified anyway because
of the added final __must_check. Because of this I still think that if
we want to *ignore* the error, we should just document and ignore it on
the caller side. That is, still modify __register_blkdev() to propagate
the error, and the caller can decide to ignore or not. In this case
we can document on fs/block_dev.c *why* we are ignoring the error.

Thoughts?

  Luis