| Message ID | 20221107150128.46951-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: queue laundrette to filecache_wq instead of system_wq | expand |
Hi Jeff - > On Nov 7, 2022, at 10:01 AM, Jeff Layton <jlayton@kernel.org> wrote: > > nfsd has grown a dedicated workqueue for the filecache, but this job is > still queued to the system_wq. Change it to use the filecache_wq. Actually... there doesn't seem to be anything special about nfsd_filecache_wq. 9542e6a643fc ("nfsd: Containerise filecache laundrette") does not make clear why it was added. Playing devil's advocate: why not make the converse change and replace it with system_wq? > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/filecache.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c > index 1e76b0d3b83a..018fd1565193 100644 > --- a/fs/nfsd/filecache.c > +++ b/fs/nfsd/filecache.c > @@ -212,7 +212,7 @@ static void > nfsd_file_schedule_laundrette(void) > { > if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags)) > - queue_delayed_work(system_wq, &nfsd_filecache_laundrette, > + queue_delayed_work(nfsd_filecache_wq, &nfsd_filecache_laundrette, > NFSD_LAUNDRETTE_DELAY); > } > > -- > 2.38.1 > -- Chuck Lever
On Mon, 2022-11-07 at 15:11 +0000, Chuck Lever III wrote: > Hi Jeff - > > > On Nov 7, 2022, at 10:01 AM, Jeff Layton <jlayton@kernel.org> wrote: > > > > nfsd has grown a dedicated workqueue for the filecache, but this job is > > still queued to the system_wq. Change it to use the filecache_wq. > > Actually... there doesn't seem to be anything special about > nfsd_filecache_wq. 9542e6a643fc ("nfsd: Containerise filecache > laundrette") does not make clear why it was added. > > Playing devil's advocate: why not make the converse change and > replace it with system_wq? > Good question. At one time, allocating a workqueue gave you a dedicated pool of threads, but that's no longer the case. Documentation/core-api/workqueue.rst says: "A wq no longer manages execution resources but serves as a domain for forward progress guarantee, flush and work item attributes." Given that we're allocating this workqueue exactly the same way we allocate the system_wq, I don't think we need our own workqueue at all. I'd be fine with changing this to be the reverse if you prefer. > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/nfsd/filecache.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c > > index 1e76b0d3b83a..018fd1565193 100644 > > --- a/fs/nfsd/filecache.c > > +++ b/fs/nfsd/filecache.c > > @@ -212,7 +212,7 @@ static void > > nfsd_file_schedule_laundrette(void) > > { > > if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags)) > > - queue_delayed_work(system_wq, &nfsd_filecache_laundrette, > > + queue_delayed_work(nfsd_filecache_wq, &nfsd_filecache_laundrette, > > NFSD_LAUNDRETTE_DELAY); > > } > > > > -- > > 2.38.1 > > > > -- > Chuck Lever > > >
> On Nov 7, 2022, at 10:55 AM, Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2022-11-07 at 15:11 +0000, Chuck Lever III wrote: >> Hi Jeff - >> >>> On Nov 7, 2022, at 10:01 AM, Jeff Layton <jlayton@kernel.org> wrote: >>> >>> nfsd has grown a dedicated workqueue for the filecache, but this job is >>> still queued to the system_wq. Change it to use the filecache_wq. >> >> Actually... there doesn't seem to be anything special about >> nfsd_filecache_wq. 9542e6a643fc ("nfsd: Containerise filecache >> laundrette") does not make clear why it was added. >> >> Playing devil's advocate: why not make the converse change and >> replace it with system_wq? >> > > Good question. > > At one time, allocating a workqueue gave you a dedicated pool of > threads, but that's no longer the case. > > Documentation/core-api/workqueue.rst says: "A wq no longer manages > execution resources but serves as a domain for forward progress > guarantee, flush and work item attributes." > > Given that we're allocating this workqueue exactly the same way we > allocate the system_wq, I don't think we need our own workqueue at all. > I'd be fine with changing this to be the reverse if you prefer. One less thing to fail on module initialization. Go for it. >>> Signed-off-by: Jeff Layton <jlayton@kernel.org> >>> --- >>> fs/nfsd/filecache.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c >>> index 1e76b0d3b83a..018fd1565193 100644 >>> --- a/fs/nfsd/filecache.c >>> +++ b/fs/nfsd/filecache.c >>> @@ -212,7 +212,7 @@ static void >>> nfsd_file_schedule_laundrette(void) >>> { >>> if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags)) >>> - queue_delayed_work(system_wq, &nfsd_filecache_laundrette, >>> + queue_delayed_work(nfsd_filecache_wq, &nfsd_filecache_laundrette, >>> NFSD_LAUNDRETTE_DELAY); >>> } >>> >>> -- >>> 2.38.1 >>> >> >> -- >> Chuck Lever >> >> >> > > -- > Jeff Layton <jlayton@kernel.org> -- Chuck Lever
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c index 1e76b0d3b83a..018fd1565193 100644 --- a/fs/nfsd/filecache.c +++ b/fs/nfsd/filecache.c @@ -212,7 +212,7 @@ static void nfsd_file_schedule_laundrette(void) { if (test_bit(NFSD_FILE_CACHE_UP, &nfsd_file_flags)) - queue_delayed_work(system_wq, &nfsd_filecache_laundrette, + queue_delayed_work(nfsd_filecache_wq, &nfsd_filecache_laundrette, NFSD_LAUNDRETTE_DELAY); }
nfsd has grown a dedicated workqueue for the filecache, but this job is still queued to the system_wq. Change it to use the filecache_wq. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/filecache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)