From: Xiubo Li <xiubli@redhat.com>
To: Jeff Layton <jlayton@kernel.org>, David Howells <dhowells@redhat.com>
Cc: Steve French <sfrench@samba.org>,
Dominique Martinet <asmadeus@codewreck.org>,
David Wysochanski <dwysocha@redhat.com>,
Ilya Dryomov <idryomov@gmail.com>,
v9fs-developer@lists.sourceforge.net, ceph-devel@vger.kernel.org,
linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org,
linux-cachefs@redhat.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] netfs: ->cleanup() op is always given a rreq pointer now
Date: Mon, 23 May 2022 09:21:23 +0800 [thread overview]
Message-ID: <72a1cb54-4632-659d-e6ec-2d754ab2fc28@redhat.com> (raw)
In-Reply-To: <e5f6fee5518ce8e1b4fc5aa7038de1617a341c2f.camel@kernel.org>
On 5/19/22 11:36 PM, Jeff Layton wrote:
> On Thu, 2022-05-19 at 15:16 +0100, David Howells wrote:
>> As the ->init() netfs op is now used to set up the netfslib I/O request
>> rather than passing stuff in, thereby allowing the netfslib functions to be
>> pointed at directly by the address_space_operations struct, we're always
>> going to be able to pass an I/O request pointer to the cleanup function.
>>
>> Therefore, change the ->cleanup() function to take a pointer to the I/O
>> request rather than taking a pointer to the network filesystem's
>> address_space and a piece of private data.
>>
>> Also, rename ->cleanup() to ->free_request() to match the ->init_request()
>> function.
>>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> cc: Jeff Layton <jlayton@kernel.org>
>> cc: Steve French <sfrench@samba.org>
>> cc: Dominique Martinet <asmadeus@codewreck.org>
>> cc: Jeff Layton <jlayton@redhat.com>
>> cc: David Wysochanski <dwysocha@redhat.com>
>> cc: Ilya Dryomov <idryomov@gmail.com>
>> cc: v9fs-developer@lists.sourceforge.net
>> cc: ceph-devel@vger.kernel.org
>> cc: linux-afs@lists.infradead.org
>> cc: linux-cifs@vger.kernel.org
>> cc: linux-cachefs@redhat.com
>> cc: linux-fsdevel@vger.kernel.org
>> ---
>>
>> fs/9p/vfs_addr.c | 11 +++++------
>> fs/afs/file.c | 6 +++---
>> fs/ceph/addr.c | 9 ++++-----
>> fs/netfs/objects.c | 8 +++++---
>> include/linux/netfs.h | 4 +++-
>> 5 files changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
>> index 501128188343..002c482794dc 100644
>> --- a/fs/9p/vfs_addr.c
>> +++ b/fs/9p/vfs_addr.c
>> @@ -66,13 +66,12 @@ static int v9fs_init_request(struct netfs_io_request *rreq, struct file *file)
>> }
>>
>> /**
>> - * v9fs_req_cleanup - Cleanup request initialized by v9fs_init_request
>> - * @mapping: unused mapping of request to cleanup
>> - * @priv: private data to cleanup, a fid, guaranted non-null.
>> + * v9fs_free_request - Cleanup request initialized by v9fs_init_rreq
>> + * @rreq: The I/O request to clean up
>> */
>> -static void v9fs_req_cleanup(struct address_space *mapping, void *priv)
>> +static void v9fs_free_request(struct netfs_io_request *rreq)
>> {
>> - struct p9_fid *fid = priv;
>> + struct p9_fid *fid = rreq->netfs_priv;
>>
>> p9_client_clunk(fid);
>> }
>> @@ -94,9 +93,9 @@ static int v9fs_begin_cache_operation(struct netfs_io_request *rreq)
>>
>> const struct netfs_request_ops v9fs_req_ops = {
>> .init_request = v9fs_init_request,
>> + .free_request = v9fs_free_request,
>> .begin_cache_operation = v9fs_begin_cache_operation,
>> .issue_read = v9fs_issue_read,
>> - .cleanup = v9fs_req_cleanup,
>> };
>>
>> /**
>> diff --git a/fs/afs/file.c b/fs/afs/file.c
>> index 26292a110a8f..b9ca72fbbcf9 100644
>> --- a/fs/afs/file.c
>> +++ b/fs/afs/file.c
>> @@ -383,17 +383,17 @@ static int afs_check_write_begin(struct file *file, loff_t pos, unsigned len,
>> return test_bit(AFS_VNODE_DELETED, &vnode->flags) ? -ESTALE : 0;
>> }
>>
>> -static void afs_priv_cleanup(struct address_space *mapping, void *netfs_priv)
>> +static void afs_free_request(struct netfs_io_request *rreq)
>> {
>> - key_put(netfs_priv);
>> + key_put(rreq->netfs_priv);
>> }
>>
>> const struct netfs_request_ops afs_req_ops = {
>> .init_request = afs_init_request,
>> + .free_request = afs_free_request,
>> .begin_cache_operation = afs_begin_cache_operation,
>> .check_write_begin = afs_check_write_begin,
>> .issue_read = afs_issue_read,
>> - .cleanup = afs_priv_cleanup,
>> };
>>
>> int afs_write_inode(struct inode *inode, struct writeback_control *wbc)
>> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
>> index b6edcf89a429..ee8c1b099c4f 100644
>> --- a/fs/ceph/addr.c
>> +++ b/fs/ceph/addr.c
>> @@ -392,11 +392,10 @@ static int ceph_init_request(struct netfs_io_request *rreq, struct file *file)
>> return 0;
>> }
>>
>> -static void ceph_readahead_cleanup(struct address_space *mapping, void *priv)
>> +static void ceph_netfs_free_request(struct netfs_io_request *rreq)
>> {
>> - struct inode *inode = mapping->host;
>> - struct ceph_inode_info *ci = ceph_inode(inode);
>> - int got = (uintptr_t)priv;
>> + struct ceph_inode_info *ci = ceph_inode(rreq->inode);
>> + int got = (uintptr_t)rreq->netfs_priv;
>>
>> if (got)
>> ceph_put_cap_refs(ci, got);
>> @@ -404,12 +403,12 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv)
>>
>> const struct netfs_request_ops ceph_netfs_ops = {
>> .init_request = ceph_init_request,
>> + .free_request = ceph_netfs_free_request,
>> .begin_cache_operation = ceph_begin_cache_operation,
>> .issue_read = ceph_netfs_issue_read,
>> .expand_readahead = ceph_netfs_expand_readahead,
>> .clamp_length = ceph_netfs_clamp_length,
>> .check_write_begin = ceph_netfs_check_write_begin,
>> - .cleanup = ceph_readahead_cleanup,
>> };
>>
>> #ifdef CONFIG_CEPH_FSCACHE
>> diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
>> index e86107b30ba4..d6b8c0cbeb7c 100644
>> --- a/fs/netfs/objects.c
>> +++ b/fs/netfs/objects.c
>> @@ -75,10 +75,10 @@ static void netfs_free_request(struct work_struct *work)
>> struct netfs_io_request *rreq =
>> container_of(work, struct netfs_io_request, work);
>>
>> - netfs_clear_subrequests(rreq, false);
>> - if (rreq->netfs_priv)
>> - rreq->netfs_ops->cleanup(rreq->mapping, rreq->netfs_priv);
>> trace_netfs_rreq(rreq, netfs_rreq_trace_free);
>> + netfs_clear_subrequests(rreq, false);
>> + if (rreq->netfs_ops->free_request)
>> + rreq->netfs_ops->free_request(rreq);
>> if (rreq->cache_resources.ops)
>> rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
>> kfree(rreq);
>> @@ -140,6 +140,8 @@ static void netfs_free_subrequest(struct netfs_io_subrequest *subreq,
>> struct netfs_io_request *rreq = subreq->rreq;
>>
>> trace_netfs_sreq(subreq, netfs_sreq_trace_free);
>> + if (rreq->netfs_ops->free_subrequest)
>> + rreq->netfs_ops->free_subrequest(subreq);
>> kfree(subreq);
>> netfs_stat_d(&netfs_n_rh_sreq);
>> netfs_put_request(rreq, was_async, netfs_rreq_trace_put_subreq);
>> diff --git a/include/linux/netfs.h b/include/linux/netfs.h
>> index c7bf1eaf51d5..1970c21b4f80 100644
>> --- a/include/linux/netfs.h
>> +++ b/include/linux/netfs.h
>> @@ -204,7 +204,10 @@ struct netfs_io_request {
>> */
>> struct netfs_request_ops {
>> int (*init_request)(struct netfs_io_request *rreq, struct file *file);
>> + void (*free_request)(struct netfs_io_request *rreq);
>> + void (*free_subrequest)(struct netfs_io_subrequest *rreq);
> Do we need free_subrequest? It looks like nothing defines it in this
> series.
If this is needed in future, or shall we do this in
netfs_clear_subrequests() ?
-- Xiubo
>> int (*begin_cache_operation)(struct netfs_io_request *rreq);
>> +
>> void (*expand_readahead)(struct netfs_io_request *rreq);
>> bool (*clamp_length)(struct netfs_io_subrequest *subreq);
>> void (*issue_read)(struct netfs_io_subrequest *subreq);
>> @@ -212,7 +215,6 @@ struct netfs_request_ops {
>> int (*check_write_begin)(struct file *file, loff_t pos, unsigned len,
>> struct folio *folio, void **_fsdata);
>> void (*done)(struct netfs_io_request *rreq);
>> - void (*cleanup)(struct address_space *mapping, void *netfs_priv);
>> };
>>
>> /*
>>
>>
next prev parent reply other threads:[~2022-05-23 1:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-19 14:16 [PATCH 1/2] netfs: ->cleanup() op is always given a rreq pointer now David Howells
2022-05-19 14:16 ` [PATCH 2/2] netfs: Export netfs_put_subrequest() David Howells
2022-05-19 15:37 ` Jeff Layton
2022-05-19 15:36 ` [PATCH 1/2] netfs: ->cleanup() op is always given a rreq pointer now Jeff Layton
2022-05-23 1:21 ` Xiubo Li [this message]
2022-05-23 6:08 ` David Howells
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=72a1cb54-4632-659d-e6ec-2d754ab2fc28@redhat.com \
--to=xiubli@redhat.com \
--cc=asmadeus@codewreck.org \
--cc=ceph-devel@vger.kernel.org \
--cc=dhowells@redhat.com \
--cc=dwysocha@redhat.com \
--cc=idryomov@gmail.com \
--cc=jlayton@kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cachefs@redhat.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sfrench@samba.org \
--cc=v9fs-developer@lists.sourceforge.net \
/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.