RE: [PATCH] fs/ceph/io: make ceph_start_io_*() killable

From: Viacheslav Dubeyko
Date: Fri Dec 06 2024 - 14:12:10 EST


On Fri, 2024-12-06 at 19:58 +0100, Max Kellermann wrote:
> On Fri, Dec 6, 2024 at 6:40 PM Viacheslav Dubeyko
> <Slava.Dubeyko@xxxxxxx> wrote:
> > Do we really need this comment (for __must_check)? It looks like
> > not
> > very informative. What do you think?
>
> That's a question of taste. For my taste, such comments are (not
> needed but) helpful; many similar comments exist in the Linux kernel.

Yeah, I completely see your point. But I believe that #include
<linux/compiler_attributes.h> is already contains enough info. If
anybody would like to understand __must_check origin, then this guy
will end into compiler_attributes.h. Otherwise, we need to comment
every #include that sounds like overkill for my taste. :)

>
> > I am not completely sure that it really needs to request compiler
> > to
> > check that return value is processed. Do we really need to enforce
> > it?
>
> Yes, should definitely be enforced. Callers which don't check the
> return value are 100% buggy.

I definitely could agree with you here. But, frankly speaking, it could
depends on function's logic. There are many places in kernel where such
checking was skipped and no harm finally. In our case, we have return
value from down_write_killable() only, mostly. Should be the check of
this function's output mandatory? I am not fully sure. But I believe
you are more right here than me.

Thanks,
Slava.