mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
To: "max.kellermann@ionos.com" <max.kellermann@ionos.com>
Cc: Alex Markuze <amarkuze@redhat.com>,
	"ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
	Xiubo Li <xiubli@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"idryomov@gmail.com" <idryomov@gmail.com>
Subject: RE: [PATCH] fs/ceph/io: make ceph_start_io_*() killable
Date: Fri, 6 Dec 2024 19:11:50 +0000	[thread overview]
Message-ID: <cd3c88aae12ad392f815c15bab0d54c8f9092e46.camel@ibm.com> (raw)
In-Reply-To: <CAKPOu+-6SfZWQTazTP_0ipnd=S0ONx8vxe070wYgakB-g_igDg@mail.gmail.com>

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



  reply	other threads:[~2024-12-06 19:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06 16:50 Max Kellermann
2024-12-06 17:40 ` Viacheslav Dubeyko
2024-12-06 18:58   ` Max Kellermann
2024-12-06 19:11     ` Viacheslav Dubeyko [this message]
2024-12-06 22:48       ` Max Kellermann
2024-12-09 18:59         ` Viacheslav Dubeyko
2025-05-19 10:15 ` Max Kellermann
2025-06-06 17:08   ` Ilya Dryomov
2025-06-06 17:15     ` Viacheslav Dubeyko
2025-06-06 17:34       ` Max Kellermann
2025-06-06 17:42         ` Viacheslav Dubeyko
2025-06-09 13:13           ` Ilya Dryomov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd3c88aae12ad392f815c15bab0d54c8f9092e46.camel@ibm.com \
    --to=slava.dubeyko@ibm.com \
    --cc=amarkuze@redhat.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=max.kellermann@ionos.com \
    --cc=xiubli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®