linux-kernel - RE: [PATCH] fs/ceph/io: make ceph_start_io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cd3c88aae12ad392f815c15bab0d54c8f9092e46.camel@ibm.com>
Date: Fri, 6 Dec 2024 19:11:50 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "max.kellermann@...os.com" <max.kellermann@...os.com>
CC: Alex Markuze <amarkuze@...hat.com>,
        "ceph-devel@...r.kernel.org"
	<ceph-devel@...r.kernel.org>,
        Xiubo Li <xiubli@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "idryomov@...il.com" <idryomov@...il.com>
Subject: RE: [PATCH] fs/ceph/io: make ceph_start_io_*() killable

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@....com> 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.