Received-SPF: pass (google.com: domain of linux-kernel+bounces-87280-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240228181850.5895-1-apais@linux.microsoft.com> <CAJhGHyD8cruwAkr+hqysv7fTQX1ovpn2cu_wU2C4TXrE5Qy8zg@mail.gmail.com>
In-Reply-To: <CAJhGHyD8cruwAkr+hqysv7fTQX1ovpn2cu_wU2C4TXrE5Qy8zg@mail.gmail.com>
From: Allen <allen.lkml@gmail.com>
Date: Thu, 29 Feb 2024 10:24:33 -0800
Message-ID: <CAOMdWSL2RTBin-FHWEv7b_1FRYs-KUztk7Ev5vaLSNe=zECuDA@mail.gmail.com>
Subject: Re: [PATCH] kernel: Introduce enable_and_queue_work() convenience function
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: Allen Pais <apais@linux.microsoft.com>, tj@kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

> >
> > +/**
> > + * enable_and_queue_work - Enable and queue a work item on a specific workqueue
> > + * @wq: The target workqueue
> > + * @work: The work item to be enabled and queued
> > + *
> > + * This function attempts to enable the specified work item using enable_work().
> > + * If the enable operation is successful, the work item is then queued on the
>
> Could you please specify what "successful" means and also please
> document it that it might cause unnecessary spurious wake-ups and
> the caller should prepare for it if it is not desired.

 Thank you for the review. I will work on documenting it.

Thanks.
> Thanks
> Lai
>
> PS:
>
> I'm afraid it can cause unnecessary spurious wake-ups in cases
> where the work item is expected to be dormant ordinarily but disable/enable()
> are called often for maintenance.  However, the user can resort to other
> synchronization in this case rather than just disable/enable() only to avoid the
> wake-ups overheads.
>
>
>
> > + * provided workqueue using queue_work(). It returns %true if the work item was
> > + * successfully enabled and queued, and %false otherwise.
> > + *
> > + * This function combines the operations of enable_work() and queue_work(),
> > + * providing a convenient way to enable and queue a work item in a single call.
> > + */
> > +static inline bool enable_and_queue_work(struct workqueue_struct *wq,
> > +                                        struct work_struct *work)
> > +{
> > +       if (enable_work(work)) {
> > +               queue_work(wq, work);
> > +               return true;
> > +       }
> > +       return false;
> > +}
> > +
> >  /*
> >   * Detect attempt to flush system-wide workqueues at compile time when possible.
> >   * Warn attempt to flush system-wide workqueues at runtime.
> > --
> > 2.17.1
> >
>


-- 
       - Allen