Yes, It will look better. Agree with your idea. I will work on it.
Regards,
Rohith
On Mon, Feb 28, 2022 at 8:09 PM David Howells <dhowells@xxxxxxxxxx> wrote:
>
> Rohith Surabattula <rohiths.msft@xxxxxxxxx> wrote:
>
> > > Rohith Surabattula <rohiths.msft@xxxxxxxxx> wrote:
> > >
> > > > + credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
> > > > ...
> > > > + subreq->subreq_priv = credits;
> > >
> > > Would it be better if I made it so that the netfs could specify the size of
> > > the netfs_read_subrequest struct to be allocated, thereby allowing it to tag
> > > extra data on the end?
> >
> > Do you mean the clamp handler in netfs should return the size of data
> > to be allocated instead of allocating itself ?
>
> No, I was thinking of putting a size_t in struct netfs_request_ops that
> indicates how big the subrequest struct should be:
>
> struct netfs_request_ops {
> ...
> size_t subrequest_size;
> };
>
> and then:
>
> struct netfs_read_subrequest *netfs_alloc_subrequest(
> struct netfs_read_request *rreq)
> {
> struct netfs_read_subrequest *subreq;
>
> subreq = kzalloc(rreq->ops->subrequest_size, GFP_KERNEL);
> if (subreq) {
> INIT_LIST_HEAD(&subreq->rreq_link);
> refcount_set(&subreq->usage, 2);
> subreq->rreq = rreq;
> netfs_get_read_request(rreq);
> netfs_stat(&netfs_n_rh_sreq);
> }
>
> return subreq;
> }
>
> This would allow you to do, for instance:
>
> struct cifs_subrequest {
> struct netfs_read_subrequest subreq;
> struct cifs_credits credits;
> };
>
> then:
>
> const struct netfs_request_ops cifs_req_ops = {
> .subrequest_size = sizeof(struct cifs_subrequest),
> .init_rreq = cifs_init_rreq,
> .expand_readahead = cifs_expand_readahead,
> .clamp_length = cifs_clamp_length,
> .issue_op = cifs_req_issue_op,
> .done = cifs_rreq_done,
> .cleanup = cifs_req_cleanup,
> };
>
> and then:
>
> static bool cifs_clamp_length(struct netfs_read_subrequest *subreq)
> {
> struct cifs_subrequest *cifs_subreq =
> container_of(subreq, struct cifs_subrequest, subreq);
> struct cifs_sb_info *cifs_sb = CIFS_SB(subreq->rreq->inode->i_sb);
> struct TCP_Server_Info *server;
> struct cifsFileInfo *open_file = subreq->rreq->netfs_priv;
> struct cifs_credits *credits = &cifs_subreq->credits;
> unsigned int rsize;
> int rc;
>
> server = cifs_pick_channel(tlink_tcon(open_file->tlink)->ses);
>
> rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize,
> &rsize, credits);
> if (rc)
> return false;
>
> subreq->len = rsize;
> return true;
> }
>
> David
>
