From: Sagi Grimberg <sagi@grimberg.me> To: Roman Pen <roman.penyaev@profitbricks.com>, linux-block@vger.kernel.org, linux-rdma@vger.kernel.org Cc: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@infradead.org>, Bart Van Assche <bart.vanassche@sandisk.com>, Or Gerlitz <ogerlitz@mellanox.com>, Danil Kipnis <danil.kipnis@profitbricks.com>, Jack Wang <jinpu.wang@profitbricks.com> Subject: Re: [PATCH 04/24] ibtrs: client: private header with client structs and functions Date: Mon, 5 Feb 2018 12:59:56 +0200 [thread overview] Message-ID: <ed312357-d43f-3a2a-52bc-f2708072126f@grimberg.me> (raw) In-Reply-To: <20180202140904.2017-5-roman.penyaev@profitbricks.com> Hi Roman, > +struct ibtrs_clt_io_req { > + struct list_head list; > + struct ibtrs_iu *iu; > + struct scatterlist *sglist; /* list holding user data */ > + unsigned int sg_cnt; > + unsigned int sg_size; > + unsigned int data_len; > + unsigned int usr_len; > + void *priv; > + bool in_use; > + struct ibtrs_clt_con *con; > + union { > + struct ib_pool_fmr **fmr_list; > + struct ibtrs_fr_desc **fr_list; > + }; We are pretty much stuck with fmrs for legacy devices, it has no future support plans, please don't add new dependencies on it. Its already hard enough to get rid of it. > + void *map_page; > + struct ibtrs_tag *tag; Can I ask why do you need another tag that is not the request tag? > + u16 nmdesc; > + enum dma_data_direction dir; > + ibtrs_conf_fn *conf; > + unsigned long start_time; > +}; > + > +static inline struct ibtrs_clt_con *to_clt_con(struct ibtrs_con *c) > +{ > + if (unlikely(!c)) > + return NULL; > + > + return container_of(c, struct ibtrs_clt_con, c); > +} > + > +static inline struct ibtrs_clt_sess *to_clt_sess(struct ibtrs_sess *s) > +{ > + if (unlikely(!s)) > + return NULL; > + > + return container_of(s, struct ibtrs_clt_sess, s); > +} Seems a bit awkward that container_of wrappers check pointer validity... > +/** > + * list_next_or_null_rr - get next list element in round-robin fashion. > + * @pos: entry, starting cursor. > + * @head: head of the list to examine. This list must have at least one > + * element, namely @pos. > + * @member: name of the list_head structure within typeof(*pos). > + * > + * Important to understand that @pos is a list entry, which can be already > + * removed using list_del_rcu(), so if @head has become empty NULL will be > + * returned. Otherwise next element is returned in round-robin fashion. > + */ > +#define list_next_or_null_rcu_rr(pos, head, member) ({ \ > + typeof(pos) ________next = NULL; \ > + \ > + if (!list_empty(head)) \ > + ________next = (pos)->member.next != (head) ? \ > + list_entry_rcu((pos)->member.next, \ > + typeof(*pos), member) : \ > + list_entry_rcu((pos)->member.next->next, \ > + typeof(*pos), member); \ > + ________next; \ > +}) Why is this local to your driver? > + > +/* See ibtrs-log.h */ > +#define TYPES_TO_SESSNAME(obj) \ > + LIST(CASE(obj, struct ibtrs_clt_sess *, s.sessname), \ > + CASE(obj, struct ibtrs_clt *, sessname)) > + > +#define TAG_SIZE(clt) (sizeof(struct ibtrs_tag) + (clt)->pdu_sz) > +#define GET_TAG(clt, idx) ((clt)->tags + TAG_SIZE(clt) * idx) Still don't understand why this is even needed..
WARNING: multiple messages have this Message-ID (diff)
From: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org> To: Roman Pen <roman.penyaev-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>, linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>, Danil Kipnis <danil.kipnis-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>, Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org> Subject: Re: [PATCH 04/24] ibtrs: client: private header with client structs and functions Date: Mon, 5 Feb 2018 12:59:56 +0200 [thread overview] Message-ID: <ed312357-d43f-3a2a-52bc-f2708072126f@grimberg.me> (raw) In-Reply-To: <20180202140904.2017-5-roman.penyaev-EIkl63zCoXaH+58JC4qpiA@public.gmane.org> Hi Roman, > +struct ibtrs_clt_io_req { > + struct list_head list; > + struct ibtrs_iu *iu; > + struct scatterlist *sglist; /* list holding user data */ > + unsigned int sg_cnt; > + unsigned int sg_size; > + unsigned int data_len; > + unsigned int usr_len; > + void *priv; > + bool in_use; > + struct ibtrs_clt_con *con; > + union { > + struct ib_pool_fmr **fmr_list; > + struct ibtrs_fr_desc **fr_list; > + }; We are pretty much stuck with fmrs for legacy devices, it has no future support plans, please don't add new dependencies on it. Its already hard enough to get rid of it. > + void *map_page; > + struct ibtrs_tag *tag; Can I ask why do you need another tag that is not the request tag? > + u16 nmdesc; > + enum dma_data_direction dir; > + ibtrs_conf_fn *conf; > + unsigned long start_time; > +}; > + > +static inline struct ibtrs_clt_con *to_clt_con(struct ibtrs_con *c) > +{ > + if (unlikely(!c)) > + return NULL; > + > + return container_of(c, struct ibtrs_clt_con, c); > +} > + > +static inline struct ibtrs_clt_sess *to_clt_sess(struct ibtrs_sess *s) > +{ > + if (unlikely(!s)) > + return NULL; > + > + return container_of(s, struct ibtrs_clt_sess, s); > +} Seems a bit awkward that container_of wrappers check pointer validity... > +/** > + * list_next_or_null_rr - get next list element in round-robin fashion. > + * @pos: entry, starting cursor. > + * @head: head of the list to examine. This list must have at least one > + * element, namely @pos. > + * @member: name of the list_head structure within typeof(*pos). > + * > + * Important to understand that @pos is a list entry, which can be already > + * removed using list_del_rcu(), so if @head has become empty NULL will be > + * returned. Otherwise next element is returned in round-robin fashion. > + */ > +#define list_next_or_null_rcu_rr(pos, head, member) ({ \ > + typeof(pos) ________next = NULL; \ > + \ > + if (!list_empty(head)) \ > + ________next = (pos)->member.next != (head) ? \ > + list_entry_rcu((pos)->member.next, \ > + typeof(*pos), member) : \ > + list_entry_rcu((pos)->member.next->next, \ > + typeof(*pos), member); \ > + ________next; \ > +}) Why is this local to your driver? > + > +/* See ibtrs-log.h */ > +#define TYPES_TO_SESSNAME(obj) \ > + LIST(CASE(obj, struct ibtrs_clt_sess *, s.sessname), \ > + CASE(obj, struct ibtrs_clt *, sessname)) > + > +#define TAG_SIZE(clt) (sizeof(struct ibtrs_tag) + (clt)->pdu_sz) > +#define GET_TAG(clt, idx) ((clt)->tags + TAG_SIZE(clt) * idx) Still don't understand why this is even needed.. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-02-05 10:59 UTC|newest] Thread overview: 124+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-02-02 14:08 [PATCH 00/24] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD) Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 01/24] ibtrs: public interface header to establish RDMA connections Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 02/24] ibtrs: private headers with IBTRS protocol structs and helpers Roman Pen 2018-02-02 14:08 ` [PATCH 03/24] ibtrs: core: lib functions shared between client and server modules Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-05 10:52 ` Sagi Grimberg 2018-02-06 12:01 ` Roman Penyaev 2018-02-06 16:10 ` Jason Gunthorpe 2018-02-07 10:34 ` Roman Penyaev 2018-02-07 10:34 ` Roman Penyaev 2018-02-02 14:08 ` [PATCH 04/24] ibtrs: client: private header with client structs and functions Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-05 10:59 ` Sagi Grimberg [this message] 2018-02-05 10:59 ` Sagi Grimberg 2018-02-06 12:23 ` Roman Penyaev 2018-02-02 14:08 ` [PATCH 05/24] ibtrs: client: main functionality Roman Pen 2018-02-02 16:54 ` Bart Van Assche 2018-02-02 16:54 ` Bart Van Assche 2018-02-05 13:27 ` Roman Penyaev 2018-02-05 14:14 ` Sagi Grimberg 2018-02-05 14:14 ` Sagi Grimberg 2018-02-05 17:05 ` Roman Penyaev 2018-02-05 17:05 ` Roman Penyaev 2018-02-05 11:19 ` Sagi Grimberg 2018-02-05 14:19 ` Roman Penyaev 2018-02-05 14:19 ` Roman Penyaev 2018-02-05 16:24 ` Bart Van Assche 2018-02-05 16:24 ` Bart Van Assche 2018-02-02 14:08 ` [PATCH 06/24] ibtrs: client: statistics functions Roman Pen 2018-02-02 14:08 ` [PATCH 07/24] ibtrs: client: sysfs interface functions Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-05 11:20 ` Sagi Grimberg 2018-02-05 11:20 ` Sagi Grimberg 2018-02-06 12:28 ` Roman Penyaev 2018-02-02 14:08 ` [PATCH 08/24] ibtrs: server: private header with server structs and functions Roman Pen 2018-02-02 14:08 ` [PATCH 09/24] ibtrs: server: main functionality Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-05 11:29 ` Sagi Grimberg 2018-02-05 11:29 ` Sagi Grimberg 2018-02-06 12:46 ` Roman Penyaev 2018-02-02 14:08 ` [PATCH 10/24] ibtrs: server: statistics functions Roman Pen 2018-02-02 14:08 ` [PATCH 11/24] ibtrs: server: sysfs interface functions Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 12/24] ibtrs: include client and server modules into kernel compilation Roman Pen 2018-02-02 14:08 ` [PATCH 13/24] ibtrs: a bit of documentation Roman Pen 2018-02-02 14:08 ` [PATCH 14/24] ibnbd: private headers with IBNBD protocol structs and helpers Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 15/24] ibnbd: client: private header with client structs and functions Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 16/24] ibnbd: client: main functionality Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 15:11 ` Jens Axboe 2018-02-02 15:11 ` Jens Axboe 2018-02-05 12:54 ` Roman Penyaev 2018-02-02 14:08 ` [PATCH 17/24] ibnbd: client: sysfs interface functions Roman Pen 2018-02-02 14:08 ` Roman Pen 2018-02-02 14:08 ` [PATCH 18/24] ibnbd: server: private header with server structs and functions Roman Pen 2018-02-02 14:08 ` [PATCH 19/24] ibnbd: server: main functionality Roman Pen 2018-02-02 14:09 ` [PATCH 20/24] ibnbd: server: functionality for IO submission to file or block dev Roman Pen 2018-02-02 14:09 ` Roman Pen 2018-02-02 14:09 ` [PATCH 21/24] ibnbd: server: sysfs interface functions Roman Pen 2018-02-02 14:09 ` Roman Pen 2018-02-02 14:09 ` [PATCH 22/24] ibnbd: include client and server modules into kernel compilation Roman Pen 2018-02-02 14:09 ` [PATCH 23/24] ibnbd: a bit of documentation Roman Pen 2018-02-02 14:09 ` Roman Pen 2018-02-02 15:55 ` Bart Van Assche 2018-02-02 15:55 ` Bart Van Assche 2018-02-05 13:03 ` Roman Penyaev 2018-02-05 14:16 ` Sagi Grimberg 2018-02-02 14:09 ` [PATCH 24/24] MAINTAINERS: Add maintainer for IBNBD/IBTRS modules Roman Pen 2018-02-02 16:07 ` [PATCH 00/24] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD) Bart Van Assche 2018-02-02 16:07 ` Bart Van Assche 2018-02-02 16:40 ` Doug Ledford 2018-02-02 16:40 ` Doug Ledford 2018-02-05 8:45 ` Jinpu Wang 2018-02-05 8:45 ` Jinpu Wang 2018-06-04 12:14 ` Danil Kipnis 2018-02-02 17:05 ` Bart Van Assche 2018-02-02 17:05 ` Bart Van Assche 2018-02-05 8:56 ` Jinpu Wang 2018-02-05 11:36 ` Sagi Grimberg 2018-02-05 13:38 ` Danil Kipnis 2018-02-05 13:38 ` Danil Kipnis 2018-02-05 14:17 ` Sagi Grimberg 2018-02-05 16:40 ` Danil Kipnis 2018-02-05 18:38 ` Bart Van Assche 2018-02-06 9:44 ` Danil Kipnis 2018-02-06 9:44 ` Danil Kipnis 2018-02-06 15:35 ` Bart Van Assche 2018-02-06 15:35 ` Bart Van Assche 2018-02-05 16:16 ` Bart Van Assche 2018-02-05 16:16 ` Bart Van Assche 2018-02-05 16:36 ` Jinpu Wang 2018-02-05 16:36 ` Jinpu Wang 2018-02-07 16:35 ` Christopher Lameter 2018-02-07 17:18 ` Roman Penyaev 2018-02-07 17:32 ` Bart Van Assche 2018-02-07 17:32 ` Bart Van Assche 2018-02-08 17:38 ` Danil Kipnis 2018-02-08 17:38 ` Danil Kipnis 2018-02-08 18:09 ` Bart Van Assche 2018-02-08 18:09 ` Bart Van Assche 2018-06-04 12:27 ` Danil Kipnis 2018-02-05 12:16 ` Sagi Grimberg 2018-02-05 12:16 ` Sagi Grimberg 2018-02-05 12:30 ` Sagi Grimberg 2018-02-07 13:06 ` Roman Penyaev 2018-02-07 13:06 ` Roman Penyaev 2018-02-05 16:58 ` Bart Van Assche 2018-02-05 16:58 ` Bart Van Assche 2018-02-05 17:16 ` Roman Penyaev 2018-02-05 17:20 ` Bart Van Assche 2018-02-05 17:20 ` Bart Van Assche 2018-02-06 11:47 ` Roman Penyaev 2018-02-06 13:12 ` Roman Penyaev 2018-02-06 13:12 ` Roman Penyaev 2018-02-06 16:01 ` Bart Van Assche 2018-02-06 16:01 ` Bart Van Assche 2018-02-07 12:57 ` Roman Penyaev 2018-02-07 12:57 ` Roman Penyaev 2018-02-07 16:35 ` Bart Van Assche 2018-02-07 16:35 ` Bart Van Assche
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=ed312357-d43f-3a2a-52bc-f2708072126f@grimberg.me \ --to=sagi@grimberg.me \ --cc=axboe@kernel.dk \ --cc=bart.vanassche@sandisk.com \ --cc=danil.kipnis@profitbricks.com \ --cc=hch@infradead.org \ --cc=jinpu.wang@profitbricks.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-rdma@vger.kernel.org \ --cc=ogerlitz@mellanox.com \ --cc=roman.penyaev@profitbricks.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: linkBe 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.