From: David Laight <David.Laight@ACULAB.COM>
To: 'Dan Williams' <dan.j.williams@intel.com>,
Lukas Wunner <lukas@wunner.de>
Cc: "'ira.weiny@intel.com'" <ira.weiny@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Bjorn Helgaas <helgaas@kernel.org>,
Gregory Price <gregory.price@memverge.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Vishal Verma <vishal.l.verma@intel.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>
Subject: RE: [PATCH V2] PCI/DOE: Detect on stack work items automatically
Date: Tue, 22 Nov 2022 22:06:04 +0000 [thread overview]
Message-ID: <f2beb27f87d44ae180d1fe9f040442a0@AcuMS.aculab.com> (raw)
In-Reply-To: <637d2f98db8b_12cdff29464@dwillia2-xfh.jf.intel.com.notmuch>
From: Dan Williams
> Sent: 22 November 2022 20:23
...
> > > > > Lukas pointed out that object_is_on_stack() is available to detect this
> > > > > automatically.
> > > > >
> > > > > Use object_is_on_stack() to determine the correct init work function to
> > > > > call.
> > > >
> > > > This is all a bit strange.
> > > > The 'onstack' flag is needed for the diagnostic check:
> > > > is_on_stack = object_is_on_stack(addr);
> > > > if (is_on_stack == onstack)
> > > > return;
> > > > pr_warn(...);
> > > > WARN_ON(1);
> > > >
> > > > So setting the flag to the location of the buffer just subverts the check.
> > > > It that is sane there ought to be a proper way to do it.
> > >
> > > If object_is_on_stack() is sufficient to check whether a struct
> > > is on the stack or not, why doesn't __init_work() use it to
> > > auto-detect whether to call debug_object_init_on_stack() or
> > > debug_object_init()?
> > >
> > > Forcing developers to use a specific initializer for something
> > > that can be auto-detected is akin to treating them like kids
> > > and telling them "You didn't say the magic word."
> > >
> > > What's the point?
>
> I had this initial reaction as well, but INIT_WORK_ONSTACK() documents
> an important detail of the object's lifetime. Here are 2 examples of
> functions that would become trickier to read if the kernel did a
> global s/INIT_WORK_ONSTACK()/INIT_WORK()/
>
> synchronize_rcu_expedited_queue_work()
> insert_wq_barrier()
>
> ...where those take arguments that are known to come from the stack and
> be used in async context.
I suspect the check was added in response to some code that added
on on-stack item and then slept after returning from that function.
One option would be to change the diagnostic check to:
is_on_stack != !object_is_on_stack(addr)
and then pass in 2 so the test always succeeds.
But I suspect that won't be liked.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
prev parent reply other threads:[~2022-11-22 22:06 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-18 0:05 [PATCH V2] PCI/DOE: Detect on stack work items automatically ira.weiny
2022-11-18 0:07 ` Ira Weiny
2022-11-18 0:50 ` Bjorn Helgaas
2022-11-18 9:20 ` David Laight
2022-11-18 18:14 ` Dan Williams
2022-11-18 18:43 ` Ira Weiny
2022-11-18 19:46 ` Dan Williams
2022-11-19 2:24 ` Li, Ming
2022-11-19 5:11 ` Dan Williams
2022-11-19 17:27 ` Ira Weiny
2022-11-22 17:13 ` Lukas Wunner
2022-11-22 17:14 ` Lukas Wunner
2022-11-22 20:22 ` Dan Williams
2022-11-22 22:06 ` David Laight [this message]
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=f2beb27f87d44ae180d1fe9f040442a0@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=gregory.price@memverge.com \
--cc=helgaas@kernel.org \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=tglx@linutronix.de \
--cc=vishal.l.verma@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.