* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) @ 2015-12-08 21:00 Or Gerlitz [not found] ` <CAJ3xEMj2EOxX1CiA93MPOsM-FdU9ijcWCYm9tObbkLqqja0PoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-08 21:00 UTC (permalink / raw) To: Doug Ledford Cc: Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Tue, Dec 8, 2015 at 8:17 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > > On 12/08/2015 01:13 PM, Sagi Grimberg wrote: > > > I mentioned this in v1. This patch set is applied over Christoph's > > device attributes patch. Will it go in as well? > > No, that's too big and not the right sort of fix for 4.4-rc. I had it > on my radar for getting into for-next. Doug, re the device attribute patch, I have expressed my opinion that --- we should be going in a slightly different direction of stashing a struct ib_device_attr as a direct or pointer field of struct ib_device and have the device or the core to fill it up just before the device instance creation with the core is to be complete. This way we can remove all the annoying calls from ULPs to query device and avoid adding so many fields to the device structure itself. So there are two suggestions on the table here and we need to hear the maintainer thinking. I do expect that your response will not be "I applied X" but rather you'll allow the time to try and convince you on whatever direction, this is pending for long time and we're on 4.4-rc4, there should be no rush to cut this over night and push to k.o the code for this small cleanup. I didn't submit a patch that follows my suggestion, but Ira did so in one of the OPA submission rounds, if you're OK with that proposal or if you even just want to see the patch to cast your vote/decision, I will rebase it and complete. Makes sense? Or. > > So, it'll have to be fixed up to apply over the top of your patches which will already be applied. > -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CAJ3xEMj2EOxX1CiA93MPOsM-FdU9ijcWCYm9tObbkLqqja0PoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <CAJ3xEMj2EOxX1CiA93MPOsM-FdU9ijcWCYm9tObbkLqqja0PoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-12-08 22:04 ` Doug Ledford [not found] ` <566753E3.9060301-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-08 22:04 UTC (permalink / raw) To: Or Gerlitz Cc: Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz [-- Attachment #1: Type: text/plain, Size: 1871 bytes --] On 12/08/2015 04:00 PM, Or Gerlitz wrote: > On Tue, Dec 8, 2015 at 8:17 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: >> >> On 12/08/2015 01:13 PM, Sagi Grimberg wrote: > > >> >>> I mentioned this in v1. This patch set is applied over Christoph's >>> device attributes patch. Will it go in as well? >> >> No, that's too big and not the right sort of fix for 4.4-rc. I had it >> on my radar for getting into for-next. > > > > Doug, re the device attribute patch, I have expressed my opinion that > --- we should be going in a slightly different direction of stashing a > struct ib_device_attr as a direct or pointer field of struct ib_device > and have the device or the core to fill it up just before the device > instance creation with the core is to be complete. This way we can > remove all the annoying calls from ULPs to query device and avoid > adding so many fields to the device structure itself. > > So there are two suggestions on the table here and we need to hear the > maintainer thinking. I do expect that your response will not be "I > applied X" but rather you'll allow the time to try and convince you on > whatever direction, this is pending for long time and we're on > 4.4-rc4, there should be no rush to cut this over night and push to > k.o the code for this small cleanup. > > I didn't submit a patch that follows my suggestion, but Ira did so in > one of the OPA submission rounds, if you're OK with that proposal or > if you even just want to see the patch to cast your vote/decision, I > will rebase it and complete. > > Makes sense? Makes sense. Show me what you are talking about (either a link to Ira's patch you are referring to or your own patch). -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <566753E3.9060301-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <566753E3.9060301-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2015-12-08 22:47 ` Or Gerlitz [not found] ` <CAJ3xEMhaEnv9He7N5q8fFsRzy_j27wdE6KWSFF39UzA680udwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-08 22:47 UTC (permalink / raw) To: Doug Ledford Cc: Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > Makes sense. thanks. > Show me what you are talking about (either a link to Ira's > patch you are referring to or your own patch). The patch is three liner to add the cached attrs -- http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK with that, I will add a 2nd patch that ports all ULPs to use the cached copy instead of their code which does the query. Actually, why not start with this approach and later decide if we need to go further of this is enough? Or. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CAJ3xEMhaEnv9He7N5q8fFsRzy_j27wdE6KWSFF39UzA680udwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <CAJ3xEMhaEnv9He7N5q8fFsRzy_j27wdE6KWSFF39UzA680udwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-12-08 22:59 ` Jason Gunthorpe [not found] ` <20151208225940.GB27609-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Jason Gunthorpe @ 2015-12-08 22:59 UTC (permalink / raw) To: Or Gerlitz Cc: Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 09, 2015 at 12:47:55AM +0200, Or Gerlitz wrote: > On Wed, Dec 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > > Makes sense. > > thanks. > > > Show me what you are talking about (either a link to Ira's > > patch you are referring to or your own patch). > > The patch is three liner to add the cached attrs -- > http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK > with that, I will add a 2nd patch that ports all ULPs to use the > cached copy instead of their code which does the query. > > Actually, why not start with this approach and later decide if we need > to go further of this is enough? Or, can we please stop this bikeshedding. Christoph's patch is done, well tested and does a lot more clean up that this hacky three liner. It is a good patch, and although patchworks doesn't have my remarks from an earlier revision I still think it should go forward. Jason -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151208225940.GB27609-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151208225940.GB27609-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2015-12-08 23:02 ` Christoph Hellwig [not found] ` <20151208230244.GA10701-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-08 23:13 ` Or Gerlitz 1 sibling, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2015-12-08 23:02 UTC (permalink / raw) To: Jason Gunthorpe Cc: Or Gerlitz, Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Tue, Dec 08, 2015 at 03:59:40PM -0700, Jason Gunthorpe wrote: > Or, can we please stop this bikeshedding. Christoph's patch is done, > well tested and does a lot more clean up that this hacky three liner. > > It is a good patch, and although patchworks doesn't have my remarks > from an earlier revision I still think it should go forward. While I'd prefer the version Or points to over not having anything at all I'd much prefer sorting it properly and make the RDMA code behave like all other Linux subsystems. Jason, can you give me a formal ACK'ed by and I'll rebase it on top of the current 4.4 queue so we could start the 4.5 window with it. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151208230244.GA10701-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151208230244.GA10701-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2015-12-08 23:49 ` Jason Gunthorpe 2015-12-09 0:52 ` ira.weiny 1 sibling, 0 replies; 39+ messages in thread From: Jason Gunthorpe @ 2015-12-08 23:49 UTC (permalink / raw) To: Christoph Hellwig Cc: Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Tue, Dec 08, 2015 at 03:02:44PM -0800, Christoph Hellwig wrote: > On Tue, Dec 08, 2015 at 03:59:40PM -0700, Jason Gunthorpe wrote: > > Or, can we please stop this bikeshedding. Christoph's patch is done, > > well tested and does a lot more clean up that this hacky three liner. > > > > It is a good patch, and although patchworks doesn't have my remarks > > from an earlier revision I still think it should go forward. > > While I'd prefer the version Or points to over not having anything > at all I'd much prefer sorting it properly and make the RDMA code > behave like all other Linux subsystems. *shrug* as long as we get the purge of ib_query_device and the assocaited migration of the driver code to the register function, I don't care very much what the names are either. device->max_map_per_fmr vs device->attr.max_map_per_fmr is just bikeshedding. > Jason, can you give me a formal ACK'ed by and I'll rebase it on top > of the current 4.4 queue so we could start the 4.5 window with it. I looked over the version in patchworks again, looks OK to me: Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> Thanks, Jason -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151208230244.GA10701-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-08 23:49 ` Jason Gunthorpe @ 2015-12-09 0:52 ` ira.weiny [not found] ` <20151209005203.GD16976-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 1 sibling, 1 reply; 39+ messages in thread From: ira.weiny @ 2015-12-09 0:52 UTC (permalink / raw) To: Christoph Hellwig Cc: Jason Gunthorpe, Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Tue, Dec 08, 2015 at 03:02:44PM -0800, Christoph Hellwig wrote: > On Tue, Dec 08, 2015 at 03:59:40PM -0700, Jason Gunthorpe wrote: > > Or, can we please stop this bikeshedding. Christoph's patch is done, > > well tested and does a lot more clean up that this hacky three liner. > > > > It is a good patch, and although patchworks doesn't have my remarks > > from an earlier revision I still think it should go forward. > > While I'd prefer the version Or points to over not having anything > at all I'd much prefer sorting it properly and make the RDMA code > behave like all other Linux subsystems. > > Jason, can you give me a formal ACK'ed by and I'll rebase it on top > of the current 4.4 queue so we could start the 4.5 window with it. Searching patchworks... I'm a bit worried about the size of the patch and I would like to see it split up for review. But I agree Christophs method is better long term. Christoph do you have this on github somewhere? Perhaps it is split but I'm not finding in on patchworks? Ira -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151209005203.GD16976-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151209005203.GD16976-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2015-12-09 18:42 ` Christoph Hellwig [not found] ` <20151209184235.GB4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2015-12-09 18:42 UTC (permalink / raw) To: ira.weiny Cc: Jason Gunthorpe, Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Tue, Dec 08, 2015 at 07:52:03PM -0500, ira.weiny wrote: > Searching patchworks... > > I'm a bit worried about the size of the patch and I would like to see it split > up for review. But I agree Christophs method is better long term. I'd be happy to split it up if I could see a way to split it. So if anyone has an idea you're welcome! > Christoph do you have this on github somewhere? Perhaps it is split but I'm > not finding in on patchworks? No need for github, we have much better (and older) git hosting sites :) http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151209184235.GB4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151209184235.GB4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2015-12-10 1:45 ` ira.weiny [not found] ` <20151210014556.GA32059-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2015-12-15 16:52 ` santosh shilimkar 1 sibling, 1 reply; 39+ messages in thread From: ira.weiny @ 2015-12-10 1:45 UTC (permalink / raw) To: Christoph Hellwig Cc: Jason Gunthorpe, Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 09, 2015 at 10:42:35AM -0800, Christoph Hellwig wrote: > On Tue, Dec 08, 2015 at 07:52:03PM -0500, ira.weiny wrote: > > Searching patchworks... > > > > I'm a bit worried about the size of the patch and I would like to see it split > > up for review. But I agree Christophs method is better long term. > > I'd be happy to split it up if I could see a way to split it. So if > anyone has an idea you're welcome! Well this is a ~3300 line patch which is pretty hard to review in total. > > > Christoph do you have this on github somewhere? Perhaps it is split but I'm > > not finding in on patchworks? > > No need for github, we have much better (and older) git hosting sites :) > > http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr Another nice side effect of this patch is to get rid of all the struct ib_device_attr allocations which are littered all over the ULPs. For the core, srp, ipoib, qib, hfi1 bits. Generally the rest looks fine I just did not have time to really go through it line by line. Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Doug this is going to conflict with the rdmavt work. So if you take this could you respond on the list. Thanks, Ira -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151210014556.GA32059-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151210014556.GA32059-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2015-12-10 8:27 ` Sagi Grimberg [not found] ` <56693758.90808-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-12-15 0:19 ` device attr cleanup (was: Handle mlx4 max_sge_rd correctly) Chuck Lever 1 sibling, 1 reply; 39+ messages in thread From: Sagi Grimberg @ 2015-12-10 8:27 UTC (permalink / raw) To: ira.weiny, Christoph Hellwig Cc: Jason Gunthorpe, Or Gerlitz, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz > Doug this is going to conflict with the rdmavt work. So if you take this could > you respond on the list. It will also conflict with the iser remote invalidate series. Doug it would help if you share your plans so people can rebase accordingly. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <56693758.90808-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <56693758.90808-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> @ 2015-12-10 16:07 ` Chuck Lever [not found] ` <93E3DE8A-0589-436D-A9A1-7EAC66B12739-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Chuck Lever @ 2015-12-10 16:07 UTC (permalink / raw) To: linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: ira.weiny, Christoph Hellwig, Jason Gunthorpe, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford > On Dec 10, 2015, at 3:27 AM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote: > > > >> Doug this is going to conflict with the rdmavt work. So if you take this could >> you respond on the list. > > It will also conflict with the iser remote invalidate series. > > Doug it would help if you share your plans so people can rebase > accordingly. I would be remiss not to mention that it probably also conflicts with the NFS server bi-directional RPC/RDMA series. Invasive IB core changes like this clean up are especially burdensome for me because NFS/RDMA changes do not normally go through Doug's tree, so it takes extra co-ordination. Here is a modest proposal. An obvious way to split the device attr cleanup might go like this: a. first patch: add new fields to ib_device b. then one patch for each provider to populate these fields c. then one patch for each kernel ULP to use the new fields d. then one patch for each provider to remove ->query_attr e. last patch: remove ib_device_attr from the IB core That way each provider and ULP maintainer can review and ack the portion of the changes that he or she is responsible for, and it should help make it much easier to merge with conflicting changes. Splitting it across more than one kernel release would be helpful too, IMO. a. and b. can go into 4.5, c. into 4.6, and d. and e. can go in any time after that. This adds more "process" but given the long chain of core changes now in plan, we should acknowledge how disruptive they will be, and come up with ways to make it possible to get other work done while the core maintenance work progresses. -- Chuck Lever -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <93E3DE8A-0589-436D-A9A1-7EAC66B12739-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* RE: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <93E3DE8A-0589-436D-A9A1-7EAC66B12739-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> @ 2015-12-10 16:10 ` Steve Wise 2015-12-10 17:17 ` Doug Ledford 2015-12-10 18:07 ` Jason Gunthorpe 1 sibling, 1 reply; 39+ messages in thread From: Steve Wise @ 2015-12-10 16:10 UTC (permalink / raw) To: 'Chuck Lever', linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: 'ira.weiny', 'Christoph Hellwig', 'Jason Gunthorpe', 'Or Gerlitz', 'Or Gerlitz', 'Sagi Grimberg', 'Doug Ledford' > -----Original Message----- > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Chuck Lever > Sent: Thursday, December 10, 2015 10:08 AM > To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: ira.weiny; Christoph Hellwig; Jason Gunthorpe; Or Gerlitz; Steve Wise; Or Gerlitz; Sagi Grimberg; Doug Ledford > Subject: Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) > > > > On Dec 10, 2015, at 3:27 AM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote: > > > > > > > >> Doug this is going to conflict with the rdmavt work. So if you take this could > >> you respond on the list. > > > > It will also conflict with the iser remote invalidate series. > > > > Doug it would help if you share your plans so people can rebase > > accordingly. > > I would be remiss not to mention that it probably also > conflicts with the NFS server bi-directional RPC/RDMA > series. > > Invasive IB core changes like this clean up are especially > burdensome for me because NFS/RDMA changes do not normally > go through Doug's tree, so it takes extra co-ordination. > > Here is a modest proposal. An obvious way to split the > device attr cleanup might go like this: > > a. first patch: add new fields to ib_device > b. then one patch for each provider to populate these fields > c. then one patch for each kernel ULP to use the new fields > d. then one patch for each provider to remove ->query_attr > e. last patch: remove ib_device_attr from the IB core > > That way each provider and ULP maintainer can review and > ack the portion of the changes that he or she is responsible > for, and it should help make it much easier to merge with > conflicting changes. > > Splitting it across more than one kernel release would be > helpful too, IMO. a. and b. can go into 4.5, c. into 4.6, > and d. and e. can go in any time after that. > > This adds more "process" but given the long chain of core > changes now in plan, we should acknowledge how disruptive > they will be, and come up with ways to make it possible to > get other work done while the core maintenance work > progresses. > > The approach sounds reasonable to me. Steve. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) 2015-12-10 16:10 ` Steve Wise @ 2015-12-10 17:17 ` Doug Ledford 0 siblings, 0 replies; 39+ messages in thread From: Doug Ledford @ 2015-12-10 17:17 UTC (permalink / raw) To: Steve Wise, 'Chuck Lever', linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: 'ira.weiny', 'Christoph Hellwig', 'Jason Gunthorpe', 'Or Gerlitz', 'Or Gerlitz', 'Sagi Grimberg' [-- Attachment #1: Type: text/plain, Size: 2622 bytes --] On 12/10/2015 11:10 AM, Steve Wise wrote: > > >> -----Original Message----- >> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Chuck Lever >> Sent: Thursday, December 10, 2015 10:08 AM >> To: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Cc: ira.weiny; Christoph Hellwig; Jason Gunthorpe; Or Gerlitz; Steve Wise; Or Gerlitz; Sagi Grimberg; Doug Ledford >> Subject: Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) >> >> >>> On Dec 10, 2015, at 3:27 AM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote: >>> >>> >>> >>>> Doug this is going to conflict with the rdmavt work. So if you take this could >>>> you respond on the list. >>> >>> It will also conflict with the iser remote invalidate series. >>> >>> Doug it would help if you share your plans so people can rebase >>> accordingly. >> >> I would be remiss not to mention that it probably also >> conflicts with the NFS server bi-directional RPC/RDMA >> series. >> >> Invasive IB core changes like this clean up are especially >> burdensome for me because NFS/RDMA changes do not normally >> go through Doug's tree, so it takes extra co-ordination. >> >> Here is a modest proposal. An obvious way to split the >> device attr cleanup might go like this: >> >> a. first patch: add new fields to ib_device >> b. then one patch for each provider to populate these fields >> c. then one patch for each kernel ULP to use the new fields >> d. then one patch for each provider to remove ->query_attr >> e. last patch: remove ib_device_attr from the IB core >> >> That way each provider and ULP maintainer can review and >> ack the portion of the changes that he or she is responsible >> for, and it should help make it much easier to merge with >> conflicting changes. >> >> Splitting it across more than one kernel release would be >> helpful too, IMO. a. and b. can go into 4.5, c. into 4.6, >> and d. and e. can go in any time after that. >> >> This adds more "process" but given the long chain of core >> changes now in plan, we should acknowledge how disruptive >> they will be, and come up with ways to make it possible to >> get other work done while the core maintenance work >> progresses. >> >> > > The approach sounds reasonable to me. This isn't a bad plan if we go that way. Now here's where I get to speak my mind on things (since people have been asking). -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <93E3DE8A-0589-436D-A9A1-7EAC66B12739-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2015-12-10 16:10 ` Steve Wise @ 2015-12-10 18:07 ` Jason Gunthorpe [not found] ` <20151210180703.GC21482-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 1 sibling, 1 reply; 39+ messages in thread From: Jason Gunthorpe @ 2015-12-10 18:07 UTC (permalink / raw) To: Chuck Lever Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Christoph Hellwig, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford On Thu, Dec 10, 2015 at 11:07:37AM -0500, Chuck Lever wrote: > Invasive IB core changes like this clean up are especially > burdensome for me because NFS/RDMA changes do not normally > go through Doug's tree, so it takes extra co-ordination. The ARM folks do this sort of stuff on a regular basis.. Very early on Doug prepares a topic branch with only the big change, NFS folks pull it and then pull your work. Then Doug would send the topic branch to Linus as soon as the merge window opens, then NFS would send theirs. This is alot less work overall than trying to sequence multiple patches over multiple releases.. Jason -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151210180703.GC21482-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151210180703.GC21482-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2015-12-10 23:30 ` Christoph Hellwig [not found] ` <20151210233047.GB26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2015-12-10 23:30 UTC (permalink / raw) To: Jason Gunthorpe Cc: Chuck Lever, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Christoph Hellwig, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: > The ARM folks do this sort of stuff on a regular basis.. Very early on > Doug prepares a topic branch with only the big change, NFS folks pull > it and then pull your work. Then Doug would send the topic branch to > Linus as soon as the merge window opens, then NFS would send theirs. > > This is alot less work overall than trying to sequence multiple > patches over multiple releases.. Agreed. Staging has alaways been a giant pain and things tend to never finish moving over that way if they are non-trivial enough. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151210233047.GB26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151210233047.GB26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2015-12-11 0:49 ` Chuck Lever [not found] ` <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Chuck Lever @ 2015-12-11 0:49 UTC (permalink / raw) To: Christoph Hellwig, Anna Schumaker, J. Bruce Fields Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford > On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: > > On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: >> The ARM folks do this sort of stuff on a regular basis.. Very early on >> Doug prepares a topic branch with only the big change, NFS folks pull >> it and then pull your work. Then Doug would send the topic branch to >> Linus as soon as the merge window opens, then NFS would send theirs. >> >> This is alot less work overall than trying to sequence multiple >> patches over multiple releases.. > > Agreed. Staging has alaways been a giant pain and things tend to never > finish moving over that way if they are non-trivial enough. In that case: You need to make sure you have all the right Acks. I've added Anna and Bruce to Ack the NFS-related portions. Santosh should Ack the RDS part. http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr Given the proximity to the holidays and the next merge window, Doug will need to get a properly-acked topic branch published in the next day or two so the rest of us can rebase and start testing before the relevant parties disappear for the holiday. -- Chuck Lever -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> @ 2015-12-11 6:54 ` santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA 2015-12-15 18:26 ` Anna Schumaker 2015-12-23 21:31 ` J. Bruce Fields 2 siblings, 0 replies; 39+ messages in thread From: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA @ 2015-12-11 6:54 UTC (permalink / raw) To: Chuck Lever, Christoph Hellwig, Anna Schumaker, J. Bruce Fields Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford, David S. Miller +Dave, On 12/10/15 4:49 PM, Chuck Lever wrote: > >> On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: >> >> On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: >>> The ARM folks do this sort of stuff on a regular basis.. Very early on >>> Doug prepares a topic branch with only the big change, NFS folks pull >>> it and then pull your work. Then Doug would send the topic branch to >>> Linus as soon as the merge window opens, then NFS would send theirs. >>> >>> This is alot less work overall than trying to sequence multiple >>> patches over multiple releases.. >> >> Agreed. Staging has alaways been a giant pain and things tend to never >> finish moving over that way if they are non-trivial enough. > > In that case: > > You need to make sure you have all the right Acks. I've added > Anna and Bruce to Ack the NFS-related portions. Santosh should > Ack the RDS part. > To make this flow work for RDS, Doug's topic branch needs to pulled into Dave's net-next as well. > http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr > > Given the proximity to the holidays and the next merge window, > Doug will need to get a properly-acked topic branch published > in the next day or two so the rest of us can rebase and start > testing before the relevant parties disappear for the holiday. > Regards, Santosh -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2015-12-11 6:54 ` santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA @ 2015-12-15 18:26 ` Anna Schumaker 2015-12-23 21:31 ` J. Bruce Fields 2 siblings, 0 replies; 39+ messages in thread From: Anna Schumaker @ 2015-12-15 18:26 UTC (permalink / raw) To: Christoph Hellwig, Chuck Lever Cc: J. Bruce Fields, Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford On Thu, Dec 10, 2015 at 7:49 PM, Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote: > >> On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: >> >> On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: >>> The ARM folks do this sort of stuff on a regular basis.. Very early on >>> Doug prepares a topic branch with only the big change, NFS folks pull >>> it and then pull your work. Then Doug would send the topic branch to >>> Linus as soon as the merge window opens, then NFS would send theirs. >>> >>> This is alot less work overall than trying to sequence multiple >>> patches over multiple releases.. >> >> Agreed. Staging has alaways been a giant pain and things tend to never >> finish moving over that way if they are non-trivial enough. > > In that case: > > You need to make sure you have all the right Acks. I've added > Anna and Bruce to Ack the NFS-related portions. Santosh should > Ack the RDS part. The NFS client parts look okay to me: Acked-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org> > > http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr > > Given the proximity to the holidays and the next merge window, > Doug will need to get a properly-acked topic branch published > in the next day or two so the rest of us can rebase and start > testing before the relevant parties disappear for the holiday. > > > -- > Chuck Lever > > > > -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2015-12-11 6:54 ` santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA 2015-12-15 18:26 ` Anna Schumaker @ 2015-12-23 21:31 ` J. Bruce Fields [not found] ` <20151223213116.GB29650-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> 2 siblings, 1 reply; 39+ messages in thread From: J. Bruce Fields @ 2015-12-23 21:31 UTC (permalink / raw) To: Chuck Lever Cc: Christoph Hellwig, Anna Schumaker, Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg, Doug Ledford On Thu, Dec 10, 2015 at 07:49:59PM -0500, Chuck Lever wrote: > > > On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: > > > > On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: > >> The ARM folks do this sort of stuff on a regular basis.. Very early on > >> Doug prepares a topic branch with only the big change, NFS folks pull > >> it and then pull your work. Then Doug would send the topic branch to > >> Linus as soon as the merge window opens, then NFS would send theirs. > >> > >> This is alot less work overall than trying to sequence multiple > >> patches over multiple releases.. > > > > Agreed. Staging has alaways been a giant pain and things tend to never > > finish moving over that way if they are non-trivial enough. > > In that case: > > You need to make sure you have all the right Acks. I've added > Anna and Bruce to Ack the NFS-related portions. Santosh should > Ack the RDS part. > > http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr Fine by me. > Given the proximity to the holidays and the next merge window, > Doug will need to get a properly-acked topic branch published > in the next day or two so the rest of us can rebase and start > testing before the relevant parties disappear for the holiday. What branch should I be working from? --b. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151223213116.GB29650-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>]
* Re: device attr cleanup [not found] ` <20151223213116.GB29650-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> @ 2015-12-24 3:19 ` Doug Ledford [not found] ` <567B6433.4050907-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-24 3:19 UTC (permalink / raw) To: J. Bruce Fields, Chuck Lever Cc: Christoph Hellwig, Anna Schumaker, Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Steve Wise, Or Gerlitz, Sagi Grimberg [-- Attachment #1: Type: text/plain, Size: 2139 bytes --] On 12/23/2015 04:31 PM, J. Bruce Fields wrote: > On Thu, Dec 10, 2015 at 07:49:59PM -0500, Chuck Lever wrote: >> >>> On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: >>> >>> On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: >>>> The ARM folks do this sort of stuff on a regular basis.. Very early on >>>> Doug prepares a topic branch with only the big change, NFS folks pull >>>> it and then pull your work. Then Doug would send the topic branch to >>>> Linus as soon as the merge window opens, then NFS would send theirs. >>>> >>>> This is alot less work overall than trying to sequence multiple >>>> patches over multiple releases.. >>> >>> Agreed. Staging has alaways been a giant pain and things tend to never >>> finish moving over that way if they are non-trivial enough. >> >> In that case: >> >> You need to make sure you have all the right Acks. I've added >> Anna and Bruce to Ack the NFS-related portions. Santosh should >> Ack the RDS part. >> >> http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr > > Fine by me. > >> Given the proximity to the holidays and the next merge window, >> Doug will need to get a properly-acked topic branch published >> in the next day or two so the rest of us can rebase and start >> testing before the relevant parties disappear for the holiday. > > What branch should I be working from? That patch was very intrusive (and I didn't like the change to the structure organization). An alternative patch was proposed and I took it instead. The patch I took is much less intrusive, but you might still need to adjust things slightly. I've pushed my current WIP for-next branch to my github repo: git://github.com/dledford/linux.git for-next This branch might get rebased still yet before it gets pushed to my official repo at k.o, but it is perfectly fine to check that your patches will merge with my for-next branch without conflicts. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <567B6433.4050907-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* RE: device attr cleanup [not found] ` <567B6433.4050907-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-01-05 16:46 ` Steve Wise 2016-01-05 17:32 ` Or Gerlitz 0 siblings, 1 reply; 39+ messages in thread From: Steve Wise @ 2016-01-05 16:46 UTC (permalink / raw) To: 'Doug Ledford', 'J. Bruce Fields', 'Chuck Lever' Cc: 'Christoph Hellwig', 'Anna Schumaker', 'Jason Gunthorpe', linux-rdma-u79uwXL29TY76Z2rM5mHXA, 'ira.weiny', 'Or Gerlitz', 'Or Gerlitz', 'Sagi Grimberg' > -----Original Message----- > From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Doug Ledford > Sent: Wednesday, December 23, 2015 9:19 PM > To: J. Bruce Fields; Chuck Lever > Cc: Christoph Hellwig; Anna Schumaker; Jason Gunthorpe; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; ira.weiny; Or Gerlitz; Steve Wise; Or Gerlitz; Sagi > Grimberg > Subject: Re: device attr cleanup > > On 12/23/2015 04:31 PM, J. Bruce Fields wrote: > > On Thu, Dec 10, 2015 at 07:49:59PM -0500, Chuck Lever wrote: > >> > >>> On Dec 10, 2015, at 6:30 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: > >>> > >>> On Thu, Dec 10, 2015 at 11:07:03AM -0700, Jason Gunthorpe wrote: > >>>> The ARM folks do this sort of stuff on a regular basis.. Very early on > >>>> Doug prepares a topic branch with only the big change, NFS folks pull > >>>> it and then pull your work. Then Doug would send the topic branch to > >>>> Linus as soon as the merge window opens, then NFS would send theirs. > >>>> > >>>> This is alot less work overall than trying to sequence multiple > >>>> patches over multiple releases.. > >>> > >>> Agreed. Staging has alaways been a giant pain and things tend to never > >>> finish moving over that way if they are non-trivial enough. > >> > >> In that case: > >> > >> You need to make sure you have all the right Acks. I've added > >> Anna and Bruce to Ack the NFS-related portions. Santosh should > >> Ack the RDS part. > >> > >> http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr > > > > Fine by me. > > > >> Given the proximity to the holidays and the next merge window, > >> Doug will need to get a properly-acked topic branch published > >> in the next day or two so the rest of us can rebase and start > >> testing before the relevant parties disappear for the holiday. > > > > What branch should I be working from? > > That patch was very intrusive (and I didn't like the change to the > structure organization). An alternative patch was proposed and I took > it instead. The patch I took is much less intrusive, but you might > still need to adjust things slightly. I've pushed my current WIP > for-next branch to my github repo: > > git://github.com/dledford/linux.git for-next > > This branch might get rebased still yet before it gets pushed to my > official repo at k.o, but it is perfectly fine to check that your > patches will merge with my for-next branch without conflicts. > Hey Doug, I don't see this branch. Which branch has the accepted device attr change? Steve. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup 2016-01-05 16:46 ` Steve Wise @ 2016-01-05 17:32 ` Or Gerlitz 0 siblings, 0 replies; 39+ messages in thread From: Or Gerlitz @ 2016-01-05 17:32 UTC (permalink / raw) To: Steve Wise Cc: Doug Ledford, J. Bruce Fields, Chuck Lever, Christoph Hellwig, Anna Schumaker, Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, ira.weiny, Or Gerlitz, Sagi Grimberg On Tue, Jan 5, 2016 at 6:46 PM, Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org> wrote: > Hey Doug, I don't see this branch. Which branch has the accepted device attr change? k.o/for-4.5 on Doug's kernel.org tree -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151210014556.GA32059-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2015-12-10 8:27 ` Sagi Grimberg @ 2015-12-15 0:19 ` Chuck Lever 1 sibling, 0 replies; 39+ messages in thread From: Chuck Lever @ 2015-12-15 0:19 UTC (permalink / raw) To: Christoph Hellwig Cc: ira.weiny, Jason Gunthorpe, Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz > On Dec 9, 2015, at 8:45 PM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote: > > On Wed, Dec 09, 2015 at 10:42:35AM -0800, Christoph Hellwig wrote: >> On Tue, Dec 08, 2015 at 07:52:03PM -0500, ira.weiny wrote: >>> Searching patchworks... >>> >>> I'm a bit worried about the size of the patch and I would like to see it split >>> up for review. But I agree Christophs method is better long term. >> >> I'd be happy to split it up if I could see a way to split it. So if >> anyone has an idea you're welcome! > > Well this is a ~3300 line patch which is pretty hard to review in total. > >> >>> Christoph do you have this on github somewhere? Perhaps it is split but I'm >>> not finding in on patchworks? >> >> No need for github, we have much better (and older) git hosting sites :) >> >> http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr Tested-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> With NFS/RDMA client and server. > Another nice side effect of this patch is to get rid of all the struct > ib_device_attr allocations which are littered all over the ULPs. > > For the core, srp, ipoib, qib, hfi1 bits. Generally the rest looks fine I just > did not have time to really go through it line by line. > > Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > > Doug this is going to conflict with the rdmavt work. So if you take this could > you respond on the list. -- Chuck Lever chucklever-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151209184235.GB4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-10 1:45 ` ira.weiny @ 2015-12-15 16:52 ` santosh shilimkar 1 sibling, 0 replies; 39+ messages in thread From: santosh shilimkar @ 2015-12-15 16:52 UTC (permalink / raw) To: Christoph Hellwig, ira.weiny Cc: Jason Gunthorpe, Or Gerlitz, Doug Ledford, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On 12/9/2015 10:42 AM, Christoph Hellwig wrote: > On Tue, Dec 08, 2015 at 07:52:03PM -0500, ira.weiny wrote: >> Searching patchworks... >> >> I'm a bit worried about the size of the patch and I would like to see it split >> up for review. But I agree Christophs method is better long term. > > I'd be happy to split it up if I could see a way to split it. So if > anyone has an idea you're welcome! > >> Christoph do you have this on github somewhere? Perhaps it is split but I'm >> not finding in on patchworks? > > No need for github, we have much better (and older) git hosting sites :) > > http://git.infradead.org/users/hch/rdma.git/shortlog/refs/heads/ib_device_attr net/rds/ib.c | 34 ++--- net/rds/iw.c | 23 +-- For RDS changes, Acked-by: Santosh Shilimlkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> I will try and find some time to test them out. Thanks !! -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151208225940.GB27609-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-12-08 23:02 ` Christoph Hellwig @ 2015-12-08 23:13 ` Or Gerlitz [not found] ` <CAJ3xEMiEYgsxiL6zR-Dia3Rxwriye1WHcadTmUjU7zV=ide1LA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-08 23:13 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 9, 2015 at 12:59 AM, Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote: > On Wed, Dec 09, 2015 at 12:47:55AM +0200, Or Gerlitz wrote: >> The patch is three liner to add the cached attrs -- >> http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK >> with that, I will add a 2nd patch that ports all ULPs to use the >> cached copy instead of their code which does the query. >> Actually, why not start with this approach and later decide if we need >> to go further of this is enough? > Or, can we please stop this bikeshedding. Christoph's patch is done, > well tested and does a lot more clean up that this hacky three liner. Christoph patch is here indeed, it does two things 1. remove all the ULP device attr alloc, device query, attr free hassle 2. adds tons of new fields to struct ib_device I think it just goes too much and needlessly adds tons of these new fields directly to struct ib_device where we can have them all well scoped into ib_device_attr member or pointer from struct ib_device > It is a good patch, I didn't say it's bad, I said I think we can achieve #1 above with way less drastic patch, and I'd like to hear the maintainer option, and have the chance to argu with the maintainer if needed, yours I heard, you like it, I know. > and although patchworks doesn't have my remarks > from an earlier revision I still think it should go forward. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CAJ3xEMiEYgsxiL6zR-Dia3Rxwriye1WHcadTmUjU7zV=ide1LA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <CAJ3xEMiEYgsxiL6zR-Dia3Rxwriye1WHcadTmUjU7zV=ide1LA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-12-08 23:15 ` Or Gerlitz [not found] ` <CAJ3xEMgQNMtxgTKC0zaKgy-WGugf6KwT7Ys5h3_RbN_7qd2=tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-09 18:44 ` device attr cleanup (was: Handle mlx4 max_sge_rd correctly) Christoph Hellwig 1 sibling, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-08 23:15 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 9, 2015 at 1:13 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Wed, Dec 9, 2015 at 12:59 AM, Jason Gunthorpe > <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote: >> On Wed, Dec 09, 2015 at 12:47:55AM +0200, Or Gerlitz wrote: > >>> The patch is three liner to add the cached attrs -- >>> http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK >>> with that, I will add a 2nd patch that ports all ULPs to use the >>> cached copy instead of their code which does the query. > >>> Actually, why not start with this approach and later decide if we need >>> to go further of this is enough? > >> Or, can we please stop this bikeshedding. Christoph's patch is done, >> well tested and does a lot more clean up that this hacky three liner. > > Christoph patch is here indeed, it does two things > > 1. remove all the ULP device attr alloc, device query, attr free hassle > 2. adds tons of new fields to struct ib_device > > I think it just goes too much and needlessly adds tons of these new > fields directly to struct ib_device where we can have them all well > scoped into ib_device_attr member or pointer from struct ib_device > >> It is a good patch, > > I didn't say it's bad, I said I think we can achieve #1 above with way > less drastic patch, and I'd like to hear the maintainer option, and > have the chance to argu with the maintainer if needed, yours I heard, > you like it, I know. and I will be OOO for the rest of this week, since this is in the air for ***months***, it would be fair enough to ask the maintainer not to cut it this way or another before next week, Doug, agree? Or. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CAJ3xEMgQNMtxgTKC0zaKgy-WGugf6KwT7Ys5h3_RbN_7qd2=tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: device attr cleanup [not found] ` <CAJ3xEMgQNMtxgTKC0zaKgy-WGugf6KwT7Ys5h3_RbN_7qd2=tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-12-15 19:03 ` Doug Ledford [not found] ` <56706414.8010807-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-15 19:03 UTC (permalink / raw) To: Or Gerlitz, Jason Gunthorpe Cc: Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz [-- Attachment #1: Type: text/plain, Size: 2257 bytes --] On 12/08/2015 06:15 PM, Or Gerlitz wrote: > On Wed, Dec 9, 2015 at 1:13 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> On Wed, Dec 9, 2015 at 12:59 AM, Jason Gunthorpe >> <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote: >>> On Wed, Dec 09, 2015 at 12:47:55AM +0200, Or Gerlitz wrote: >> >>>> The patch is three liner to add the cached attrs -- >>>> http://marc.info/?l=linux-rdma&m=142309296813985&w=2 -- if you are OK >>>> with that, I will add a 2nd patch that ports all ULPs to use the >>>> cached copy instead of their code which does the query. >> >>>> Actually, why not start with this approach and later decide if we need >>>> to go further of this is enough? >> >>> Or, can we please stop this bikeshedding. Christoph's patch is done, >>> well tested and does a lot more clean up that this hacky three liner. >> >> Christoph patch is here indeed, it does two things >> >> 1. remove all the ULP device attr alloc, device query, attr free hassle >> 2. adds tons of new fields to struct ib_device >> >> I think it just goes too much and needlessly adds tons of these new >> fields directly to struct ib_device where we can have them all well >> scoped into ib_device_attr member or pointer from struct ib_device >> >>> It is a good patch, >> >> I didn't say it's bad, I said I think we can achieve #1 above with way >> less drastic patch, and I'd like to hear the maintainer option, and >> have the chance to argu with the maintainer if needed, yours I heard, >> you like it, I know. > > and I will be OOO for the rest of this week, since this is in the air > for ***months***, it would be fair enough to ask the maintainer not to > cut it this way or another before next week, Doug, agree? Or, you specifically asked me to wait until this week. I made my initial impressions clear (I don't necessarily like the removal of the attr struct, but I like the removal of all of the query calls, and I'm inclined to take the patch in spite of not liking the removal of the struct). Do you have anything to add or have we beat this horse to death? -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <56706414.8010807-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup [not found] ` <56706414.8010807-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2015-12-16 5:53 ` Or Gerlitz [not found] ` <5670FC5E.8070405-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-16 5:53 UTC (permalink / raw) To: Doug Ledford Cc: Or Gerlitz, Jason Gunthorpe, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise On 12/15/2015 9:03 PM, Doug Ledford wrote: > > Or, you specifically asked me to wait until this week. I made my > initial impressions clear (I don't necessarily like the removal of the > attr struct, but I like the removal of all of the query calls, and I'm > inclined to take the patch in spite of not liking the removal of the > struct). Do you have anything to add or have we beat this horse to death? Hi Doug, Lets stop beating, both horses and people. I do understand that 1. you don't link the removal of the attr 2. you do like the removal of all the query calls I am proposing to take the path of a patch that does exactly #2 while avoiding #1. What's wrong with that? I haven't heard any reasoning for why its so good to stash ~50 new fields on the IB device structure except for the author saying that other subsystems do that and other people saying they are in favor of this approach while not providing any reasoning, except for maybe something on bikes. Why you or anyone else has to be from now and ever the cache line police making sure that people don't add new attributes in random locations over the IB device structure? What's wrong with putting fifty attributesin a structure which is a field of the device struct and have people go there to see what are the d ifferentattrs and add news ones there? This will make the 4.5 merge window extremely complex or even totally threatened w.r.t to the RDMA subsystem and related drivers by 3.3K LOC patch. Sorry, but, I still don't get it. Or. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <5670FC5E.8070405-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]
* Re: device attr cleanup [not found] ` <5670FC5E.8070405-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> @ 2015-12-16 13:40 ` Sagi Grimberg [not found] ` <567169B3.4040908-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-12-22 7:56 ` Or Gerlitz 1 sibling, 1 reply; 39+ messages in thread From: Sagi Grimberg @ 2015-12-16 13:40 UTC (permalink / raw) To: Or Gerlitz, Doug Ledford Cc: Or Gerlitz, Jason Gunthorpe, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise > Hi Doug, > > Lets stop beating, both horses and people. > > I do understand that > > 1. you don't link the removal of the attr > 2. you do like the removal of all the query calls > > I am proposing to take the path of a patch that > does exactly #2 while avoiding #1. I really don't have a strong preference on either of the approaches. I just want to see this included one way or the other. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <567169B3.4040908-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]
* Re: device attr cleanup [not found] ` <567169B3.4040908-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> @ 2015-12-16 14:03 ` Or Gerlitz 0 siblings, 0 replies; 39+ messages in thread From: Or Gerlitz @ 2015-12-16 14:03 UTC (permalink / raw) To: Sagi Grimberg, Doug Ledford Cc: Or Gerlitz, Jason Gunthorpe, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise On 12/16/2015 3:40 PM, Sagi Grimberg wrote: > I really don't have a strong preference on either of the approaches. I > just want to see this included one way or the other. sure, agree, I will send my patches tomorrow -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup [not found] ` <5670FC5E.8070405-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-12-16 13:40 ` Sagi Grimberg @ 2015-12-22 7:56 ` Or Gerlitz [not found] ` <CAJ3xEMiGOF79o1OSgxgk2tN+kY91uiNY5Aq+xu2U0Sur-htxpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 39+ messages in thread From: Or Gerlitz @ 2015-12-22 7:56 UTC (permalink / raw) To: Doug Ledford Cc: Jason Gunthorpe, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise On Wed, Dec 16, 2015 at 7:53 AM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote: > On 12/15/2015 9:03 PM, Doug Ledford wrote: >> Or, you specifically asked me to wait until this week. I made my >> initial impressions clear (I don't necessarily like the removal of the >> attr struct, but I like the removal of all of the query calls, and I'm >> inclined to take the patch in spite of not liking the removal of the >> struct). Do you have anything to add or have we beat this horse to death? > Hi Doug, > Lets stop beating, both horses and people. > I do understand that > 1. you don't link the removal of the attr > 2. you do like the removal of all the query calls > > I am proposing to take the path of a patch that > does exactly #2 while avoiding #1. Doug, Did you look on my v1 post and the related discussion there w.r.t udata? You didn't make any comment on my response here nor on the proposed patches. Since we are really short in time w.r.t EOY holidays and we have the udata matter open (see [1]), could we move finalizing this discussion to the 4.6 time-frame? If you do have the time, I think it would be fair to see a response from you on the discussion before you pick any of the two patch sets - so?? Or. [1] Christoph's patch doesn't remove the query_device callback from mlx4 since we report there values to libmlx4 through the udata mechanism. The query_device callback will need to be present in future/current drivers if they decide to use udata as well > What's wrong with that? I haven't heard any reasoning for why its > so good to stash ~50 new fields on the IB device structure except > for the author saying that other subsystems do that and other people > saying they are in favor of this approach while not providing any > reasoning, except for maybe something on bikes. > > Why you or anyone else has to be from now and ever the cache line police > making sure that people don't add new attributes in random locations > over the IB device structure? > > What's wrong with putting fifty attributesin a structure which is a field > of the device struct and have people go there to see what are the d > ifferentattrs and add news ones there? > > This will make the 4.5 merge window extremely complex or even totally > threatened w.r.t to the RDMA subsystem and related drivers by 3.3K LOC > patch. > > Sorry, but, I still don't get it. > > Or. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CAJ3xEMiGOF79o1OSgxgk2tN+kY91uiNY5Aq+xu2U0Sur-htxpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: device attr cleanup [not found] ` <CAJ3xEMiGOF79o1OSgxgk2tN+kY91uiNY5Aq+xu2U0Sur-htxpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2015-12-22 19:19 ` Doug Ledford [not found] ` <5679A254.2040809-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-22 19:19 UTC (permalink / raw) To: Or Gerlitz Cc: Jason Gunthorpe, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise [-- Attachment #1: Type: text/plain, Size: 3388 bytes --] On 12/22/2015 02:56 AM, Or Gerlitz wrote: > On Wed, Dec 16, 2015 at 7:53 AM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote: >> On 12/15/2015 9:03 PM, Doug Ledford wrote: > >>> Or, you specifically asked me to wait until this week. I made my >>> initial impressions clear (I don't necessarily like the removal of the >>> attr struct, but I like the removal of all of the query calls, and I'm >>> inclined to take the patch in spite of not liking the removal of the >>> struct). Do you have anything to add or have we beat this horse to death? > >> Hi Doug, >> Lets stop beating, both horses and people. >> I do understand that >> 1. you don't link the removal of the attr >> 2. you do like the removal of all the query calls >> >> I am proposing to take the path of a patch that >> does exactly #2 while avoiding #1. > > Doug, > > Did you look on my v1 post and the related discussion there w.r.t udata? Yes, I did. > You didn't make any comment on my response here nor on the proposed patches. I'm trying to find all of the emails, they aren't in a single thread in my mailbox (I had to do some reconstruction of my mailbox due to a problem in a mail filter late last week...missing that the rule was set to "match any" when I intended "match all" and the action of the rule was "delete" when I expected delete to be the same as "move to trash" and it wasn't, it was delete immediately, has caused me some problems). > Since we are really short in time w.r.t EOY holidays and we have the > udata matter > open (see [1]), could we move finalizing this discussion to the 4.6 time-frame? > > If you do have the time, I think it would be fair to see a response > from you on the > discussion before you pick any of the two patch sets - so?? I'm not inclined to take either patch set as they stand. Your's is closer to what I'm leaning towards though. I think I can add a single patch to yours to make it into what I want. I'm going to go work on that right now... > Or. > > [1] Christoph's patch doesn't remove the query_device callback from > mlx4 since we > report there values to libmlx4 through the udata mechanism. The > query_device callback > will need to be present in future/current drivers if they decide to > use udata as well > > >> What's wrong with that? I haven't heard any reasoning for why its >> so good to stash ~50 new fields on the IB device structure except >> for the author saying that other subsystems do that and other people >> saying they are in favor of this approach while not providing any >> reasoning, except for maybe something on bikes. >> >> Why you or anyone else has to be from now and ever the cache line police >> making sure that people don't add new attributes in random locations >> over the IB device structure? >> >> What's wrong with putting fifty attributesin a structure which is a field >> of the device struct and have people go there to see what are the d >> ifferentattrs and add news ones there? >> >> This will make the 4.5 merge window extremely complex or even totally >> threatened w.r.t to the RDMA subsystem and related drivers by 3.3K LOC >> patch. >> >> Sorry, but, I still don't get it. >> >> Or. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <5679A254.2040809-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup [not found] ` <5679A254.2040809-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2015-12-22 20:37 ` Doug Ledford 0 siblings, 0 replies; 39+ messages in thread From: Doug Ledford @ 2015-12-22 20:37 UTC (permalink / raw) To: Or Gerlitz Cc: Jason Gunthorpe, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise [-- Attachment #1: Type: text/plain, Size: 3918 bytes --] On 12/22/2015 02:19 PM, Doug Ledford wrote: > On 12/22/2015 02:56 AM, Or Gerlitz wrote: >> On Wed, Dec 16, 2015 at 7:53 AM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote: >>> On 12/15/2015 9:03 PM, Doug Ledford wrote: >> >>>> Or, you specifically asked me to wait until this week. I made my >>>> initial impressions clear (I don't necessarily like the removal of the >>>> attr struct, but I like the removal of all of the query calls, and I'm >>>> inclined to take the patch in spite of not liking the removal of the >>>> struct). Do you have anything to add or have we beat this horse to death? >> >>> Hi Doug, >>> Lets stop beating, both horses and people. >>> I do understand that >>> 1. you don't link the removal of the attr >>> 2. you do like the removal of all the query calls >>> >>> I am proposing to take the path of a patch that >>> does exactly #2 while avoiding #1. >> >> Doug, >> >> Did you look on my v1 post and the related discussion there w.r.t udata? > > Yes, I did. > >> You didn't make any comment on my response here nor on the proposed patches. > > I'm trying to find all of the emails, they aren't in a single thread in > my mailbox (I had to do some reconstruction of my mailbox due to a > problem in a mail filter late last week...missing that the rule was set > to "match any" when I intended "match all" and the action of the rule > was "delete" when I expected delete to be the same as "move to trash" > and it wasn't, it was delete immediately, has caused me some problems). OK, here's part of the problem. The udata discussion was in your original patch series, not the V1 series. I don't have that in my mailbox at all (it was a casualty of the aforementioned mailbox event, and I probably could have recovered it at the time, but I didn't think I needed the original thread, just the V1 thread, so I didn't). However, I looked things up on marc.info. It would have been preferable to either remove query_device from the hardware drivers or to rename it to something that clearly denotes it is now a kernel internal call-in point (say device->init_ib_dev_attr). Christoph's patch doesn't really get rid of each device's query_device, it just moves the code into each device's init routine and drops the call point. As far as I'm concerned, leaving the code in a function and calling that function either from each driver's init routine or from the core registration function is neither here nor there to me, either would suffice. >> Since we are really short in time w.r.t EOY holidays and we have the >> udata matter >> open (see [1]), could we move finalizing this discussion to the 4.6 time-frame? >> >> If you do have the time, I think it would be fair to see a response >> from you on the >> discussion before you pick any of the two patch sets - so?? > > I'm not inclined to take either patch set as they stand. Your's is > closer to what I'm leaning towards though. I think I can add a single > patch to yours to make it into what I want. I'm going to go work on > that right now... After looking it over in detail, I'm not going to do what I had in mind. I still think the udata issue should be resolved, but I'm willing to take that as a follow-on patch later on. So, for now, I've taken in your v1 patchset. I'm now going to start seeing how many of the patchsets I had also intended to take for 4.5 will need possible respins in order to apply cleanly :-/ >> Or. >> >> [1] Christoph's patch doesn't remove the query_device callback from >> mlx4 since we >> report there values to libmlx4 through the udata mechanism. The >> query_device callback >> will need to be present in future/current drivers if they decide to >> use udata as well -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <CAJ3xEMiEYgsxiL6zR-Dia3Rxwriye1WHcadTmUjU7zV=ide1LA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-08 23:15 ` Or Gerlitz @ 2015-12-09 18:44 ` Christoph Hellwig [not found] ` <20151209184448.GC4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 1 sibling, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2015-12-09 18:44 UTC (permalink / raw) To: Or Gerlitz Cc: Jason Gunthorpe, Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Wed, Dec 09, 2015 at 01:13:29AM +0200, Or Gerlitz wrote: > > Christoph patch is here indeed, it does two things > > 1. remove all the ULP device attr alloc, device query, attr free hassle > 2. adds tons of new fields to struct ib_device > > I think it just goes too much and needlessly adds tons of these new > fields directly to struct ib_device where we can have them all well > scoped into ib_device_attr member or pointer from struct ib_device What's the benefit of that? And looking at the existing members of struct ib_device what determines if it goes straight into the device or the attribute? There is a reason why we don't do this weird attr split in other Linux subsystems, and making IB follow this pattern makes everyone feel right at home instead of wondering about the weird attribute. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151209184448.GC4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151209184448.GC4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2015-12-10 17:46 ` Doug Ledford [not found] ` <5669BA8E.30200-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-10 17:46 UTC (permalink / raw) To: Christoph Hellwig, Or Gerlitz Cc: Jason Gunthorpe, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz [-- Attachment #1: Type: text/plain, Size: 3136 bytes --] On 12/09/2015 01:44 PM, Christoph Hellwig wrote: > On Wed, Dec 09, 2015 at 01:13:29AM +0200, Or Gerlitz wrote: >> >> Christoph patch is here indeed, it does two things >> >> 1. remove all the ULP device attr alloc, device query, attr free hassle >> 2. adds tons of new fields to struct ib_device >> >> I think it just goes too much and needlessly adds tons of these new >> fields directly to struct ib_device where we can have them all well >> scoped into ib_device_attr member or pointer from struct ib_device > > What's the benefit of that? Organization. Let's be fair, the totally flat namespace you are preferring is the equivalent of a teenager that is completely incapable of picking thier dirty laundry up off the floor. It is sloppy, disorganized, often full of old cruft that you don't know if you can get rid of or not, often so disorganized you might have three similarly named items that you can't figure out which one should be used in which circumstances, etc. The downside of a more organized approach is often longer/more complex variable names. It also means that if you want to micro-optimize for which variables are in the hot path and you need one and only one of the variables from the attr struct, then you either have to make a copy somewhere else that is in a frequently hot cache line (along with making sure the copy and the original stay in sync) or you have to take it out of the attr struct, or other bad things. > And looking at the existing members of > struct ib_device what determines if it goes straight into the device > or the attribute? Organization. What goes where depends on what makes sense according to the organization you are doing. > There is a reason why we don't do this weird > attr split in other Linux subsystems, and making IB follow this pattern > makes everyone feel right at home instead of wondering about the > weird attribute. Being organized is not "weird". Let's not wax poetic about sloppy, disorganized structures. Let's be honest about what they are so we don't feel like we need to take a shower every time we talk about them to purge us of the sins of our lies. That said, though, the kernel frequently has hot spots that require we have the freedom to rearrange structures to suit memory placement constraints. It's really for that reason more than anything else that we tolerate these horribly disorganized structures with a totally flat namespace. And to be fair, any super high speed core code like the IB code is more susceptible than most to issues of cache line delays and hot path slow downs. For that reason, and *only* that reason, I'm inclined to take your patch. Otherwise, I wouldn't touch it. The rest of the kernel may think a disorganized, flat namespace is fruit punch kool-aid...I happen to prefer things differently. But I'm willing to acquiesce to the needs of a high performance kernel subsystem as appropriate regardless of my personal taste on the issue. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <5669BA8E.30200-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <5669BA8E.30200-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2015-12-10 23:29 ` Christoph Hellwig [not found] ` <20151210232956.GA26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2015-12-10 23:29 UTC (permalink / raw) To: Doug Ledford Cc: Christoph Hellwig, Or Gerlitz, Jason Gunthorpe, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Thu, Dec 10, 2015 at 12:46:54PM -0500, Doug Ledford wrote: > Organization. Let's be fair, the totally flat namespace you are > preferring is the equivalent of a teenager that is completely incapable > of picking thier dirty laundry up off the floor. It is sloppy, > disorganized, often full of old cruft that you don't know if you can get > rid of or not, often so disorganized you might have three similarly > named items that you can't figure out which one should be used in which > circumstances, etc. The most cruft I've found in major subsystems in years has been in the RDMA code, so I'm not sure where that argument comes from. We're pretty good at garbage collecting cruft in Linux, and the typical counter examples are arbitrarily split structures where it's easy to hide things. > > And looking at the existing members of > > struct ib_device what determines if it goes straight into the device > > or the attribute? > > Organization. What goes where depends on what makes sense according to > the organization you are doing. So what makes num_comp_vectors or phys_port_cnt fit into ib_device, while max_qp or max_cq are in struct ib_device_attr? I really like clean data structures, but keeping structures that have 1:1 relationships and sit in the same module separate never has been a good idea. > > There is a reason why we don't do this weird > > attr split in other Linux subsystems, and making IB follow this pattern > > makes everyone feel right at home instead of wondering about the > > weird attribute. > > Being organized is not "weird". Let's not wax poetic about sloppy, > disorganized structures. Let's be honest about what they are so we > don't feel like we need to take a shower every time we talk about them > to purge us of the sins of our lies. I call that utter BS. Being organized is exactly not having multiple structures that have the same scope or life time, it's actually what I call disorganzied. There is a lot to be said about grouping the fields in the structure, and that's how sensible subsystems handle it: stuct foo_bar { /* read/write in the hot path, keep together and tightly packed: */ ... /* read-only in the hot path */ ... /* random members: */ ... /* properties here, immutable after setup: */ ... }; but that's completely inverse to what we're having with ib_device currently. -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151210232956.GA26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151210232956.GA26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2015-12-11 4:56 ` Doug Ledford [not found] ` <566A5792.9080102-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Doug Ledford @ 2015-12-11 4:56 UTC (permalink / raw) To: Christoph Hellwig Cc: Or Gerlitz, Jason Gunthorpe, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz [-- Attachment #1: Type: text/plain, Size: 5113 bytes --] On 12/10/2015 06:29 PM, Christoph Hellwig wrote: > On Thu, Dec 10, 2015 at 12:46:54PM -0500, Doug Ledford wrote: >> Organization. Let's be fair, the totally flat namespace you are >> preferring is the equivalent of a teenager that is completely incapable >> of picking thier dirty laundry up off the floor. It is sloppy, >> disorganized, often full of old cruft that you don't know if you can get >> rid of or not, often so disorganized you might have three similarly >> named items that you can't figure out which one should be used in which >> circumstances, etc. > > The most cruft I've found in major subsystems in years has been > in the RDMA code, so I'm not sure where that argument comes from. From reading struct netdevice and trying to work out what all of the items do. The portion I didn't like before is gone now, so it doesn't do any good for me to reference it (it was related to the address lists for the device). > We're pretty good at garbage collecting cruft in Linux, and the > typical counter examples are arbitrarily split structures where it's > easy to hide things. > >>> And looking at the existing members of >>> struct ib_device what determines if it goes straight into the device >>> or the attribute? >> >> Organization. What goes where depends on what makes sense according to >> the organization you are doing. > > So what makes num_comp_vectors or phys_port_cnt fit into ib_device, > while max_qp or max_cq are in struct ib_device_attr? Historically, the latter two were part of the device query and represented device maximums (and almost all of ib_device_attr is this sort of thing). The first depends on the initialization results for the PCI dev and is not a device limit that can be queried. The second is a high level device property that allows you to know how many devices need queried. If you were to ever have per-port struct for device capabilities, the port count would necessarily need to be outside of that area. So, from a logical perspective, ib_device_attr contents can be thought of as those things you get when you query the card, and only things you get by querying the card (although not all cards have all items in this struct), while the others in ib_device are part of other initialization steps. > I really like clean data structures, but keeping structures that > have 1:1 relationships and sit in the same module separate never > has been a good idea. Looking at struct netdevice, it has the sort of organization I would call reasonable. Things like struct tx_stats is a struct, even though it's embedded in the parent struct and not a pointer and there is exactly and only one copy, so it could be flat. >>> There is a reason why we don't do this weird >>> attr split in other Linux subsystems, and making IB follow this pattern >>> makes everyone feel right at home instead of wondering about the >>> weird attribute. >> >> Being organized is not "weird". Let's not wax poetic about sloppy, >> disorganized structures. Let's be honest about what they are so we >> don't feel like we need to take a shower every time we talk about them >> to purge us of the sins of our lies. > > I call that utter BS. Being organized is exactly not having multiple > structures that have the same scope or life time, it's actually what > I call disorganzied. While I didn't make a huge search for anything, at least the tx stats in struct netdevice are a direct contradiction to this statement. They are something I would call well organized. And speaking of that, in struct netdevice, all of the various ops elements are handled as structs, including the base netdevice ops, whereas struct ib_device puts the base ops flat in the struct. So if we wanted to be more like struct netdevice, we would move the base ops out of struct ib_device. > There is a lot to be said about grouping the > fields in the structure, and that's how sensible subsystems handle it: > > stuct foo_bar { > /* read/write in the hot path, keep together and tightly packed: */ > ... > > /* read-only in the hot path */ > ... > > /* random members: */ > ... > > /* properties here, immutable after setup: */ > ... > }; > > but that's completely inverse to what we're having with ib_device > currently. Well, the current struct ib_device is not well laid out. That I'll grant. But it's not because ib_device_attr is a struct. It simply isn't well laid out. Among other things, for the example you give above, struct ib_device is the wrong layer to be trying to lay things out like this. An ib_device can be multi-port, and each port can be a different RDMA device type. I would argue then that the hotpath items need to be done at the port level, not the parent device level. We started this with the port_immutable array. I would argue that continuing with that work and moving further down that path is the right way to go. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <566A5792.9080102-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <566A5792.9080102-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2015-12-11 6:14 ` Jason Gunthorpe [not found] ` <20151211061433.GB16513-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 39+ messages in thread From: Jason Gunthorpe @ 2015-12-11 6:14 UTC (permalink / raw) To: Doug Ledford Cc: Christoph Hellwig, Or Gerlitz, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz On Thu, Dec 10, 2015 at 11:56:50PM -0500, Doug Ledford wrote: > Looking at struct netdevice, it has the sort of organization I would > call reasonable. Things like struct tx_stats is a struct, even though > it's embedded in the parent struct and not a pointer and there is > exactly and only one copy, so it could be flat. struct net_device_stats exists because it is used in many more places that just in struct net_device, so this is a code sharing thing. > something I would call well organized. And speaking of that, in struct > netdevice, all of the various ops elements are handled as structs, .. and 'struct net_device_ops' is used extensively. Whereas once we get rid of the query call ib_device_attr has no second user. > including the base netdevice ops, whereas struct ib_device puts the base > ops flat in the struct. So if we wanted to be more like struct > netdevice, we would move the base ops out of struct ib_device. It would make the drivers a bit nicer if they initialized an ib_ops structure like netdev does instead of in code, but performance wise this is probably better, especially if we sorted the struct members sanely.. > out like this. An ib_device can be multi-port, and each port can be a > different RDMA device type. I thought we figured out that wasn't actually allowed today when working on the caps rework? Jason -- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20151211061433.GB16513-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: device attr cleanup (was: Handle mlx4 max_sge_rd correctly) [not found] ` <20151211061433.GB16513-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2015-12-11 17:10 ` Doug Ledford 0 siblings, 0 replies; 39+ messages in thread From: Doug Ledford @ 2015-12-11 17:10 UTC (permalink / raw) To: Jason Gunthorpe Cc: Christoph Hellwig, Or Gerlitz, Sagi Grimberg, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Steve Wise, Or Gerlitz [-- Attachment #1: Type: text/plain, Size: 2741 bytes --] On 12/11/2015 01:14 AM, Jason Gunthorpe wrote: > On Thu, Dec 10, 2015 at 11:56:50PM -0500, Doug Ledford wrote: > >> Looking at struct netdevice, it has the sort of organization I would >> call reasonable. Things like struct tx_stats is a struct, even though >> it's embedded in the parent struct and not a pointer and there is >> exactly and only one copy, so it could be flat. > > struct net_device_stats exists because it is used in many more places > that just in struct net_device, so this is a code sharing thing. It doubles well as decent organization. >> something I would call well organized. And speaking of that, in struct >> netdevice, all of the various ops elements are handled as structs, > > .. and 'struct net_device_ops' is used extensively. > > Whereas once we get rid of the query call ib_device_attr has no second > user. > >> including the base netdevice ops, whereas struct ib_device puts the base >> ops flat in the struct. So if we wanted to be more like struct >> netdevice, we would move the base ops out of struct ib_device. > > It would make the drivers a bit nicer if they initialized an ib_ops > structure like netdev does instead of in code, but performance wise > this is probably better, especially if we sorted the struct members > sanely.. You could still create a struct and embed the struct in ibdev and it would make reading ibdev *much* nicer while preserving this performance benefit (which is probably very minor, if at all, and very well might become a performance penalty if you have more than one ibdev in heavy use on the system). The huge list of ops in the middle of the struct is a gigantic eyesore. >> out like this. An ib_device can be multi-port, and each port can be a >> different RDMA device type. > > I thought we figured out that wasn't actually allowed today when > working on the caps rework? We figured out that the core code couldn't currently do something like iWARP and RoCE on the same port even if registered as two different ports. We do IB/RoCE on different ports because they both appear as an IB device, but I think the ongoing namespace work shows that they are diverging more over time and eventually may not work in this fashion nearly as cleanly as they did to start with. I also commented that it was entirely understandable for a person to want to do the whole iWARP/RoCE on one port via two registrations thing once both the soft-RoCE and soft-iWARP drivers were in the kernel. As such, I'd like to see us not make achieving this goal harder in case we decide to pursue it later. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2016-01-05 17:32 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-08 21:00 device attr cleanup (was: Handle mlx4 max_sge_rd correctly) Or Gerlitz [not found] ` <CAJ3xEMj2EOxX1CiA93MPOsM-FdU9ijcWCYm9tObbkLqqja0PoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-08 22:04 ` Doug Ledford [not found] ` <566753E3.9060301-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-12-08 22:47 ` Or Gerlitz [not found] ` <CAJ3xEMhaEnv9He7N5q8fFsRzy_j27wdE6KWSFF39UzA680udwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-08 22:59 ` Jason Gunthorpe [not found] ` <20151208225940.GB27609-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-12-08 23:02 ` Christoph Hellwig [not found] ` <20151208230244.GA10701-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-08 23:49 ` Jason Gunthorpe 2015-12-09 0:52 ` ira.weiny [not found] ` <20151209005203.GD16976-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2015-12-09 18:42 ` Christoph Hellwig [not found] ` <20151209184235.GB4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-10 1:45 ` ira.weiny [not found] ` <20151210014556.GA32059-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2015-12-10 8:27 ` Sagi Grimberg [not found] ` <56693758.90808-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-12-10 16:07 ` Chuck Lever [not found] ` <93E3DE8A-0589-436D-A9A1-7EAC66B12739-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2015-12-10 16:10 ` Steve Wise 2015-12-10 17:17 ` Doug Ledford 2015-12-10 18:07 ` Jason Gunthorpe [not found] ` <20151210180703.GC21482-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-12-10 23:30 ` Christoph Hellwig [not found] ` <20151210233047.GB26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-11 0:49 ` Chuck Lever [not found] ` <09F96E8A-5E1E-4345-9069-C07108AF0BC7-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2015-12-11 6:54 ` santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA 2015-12-15 18:26 ` Anna Schumaker 2015-12-23 21:31 ` J. Bruce Fields [not found] ` <20151223213116.GB29650-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> 2015-12-24 3:19 ` device attr cleanup Doug Ledford [not found] ` <567B6433.4050907-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-01-05 16:46 ` Steve Wise 2016-01-05 17:32 ` Or Gerlitz 2015-12-15 0:19 ` device attr cleanup (was: Handle mlx4 max_sge_rd correctly) Chuck Lever 2015-12-15 16:52 ` santosh shilimkar 2015-12-08 23:13 ` Or Gerlitz [not found] ` <CAJ3xEMiEYgsxiL6zR-Dia3Rxwriye1WHcadTmUjU7zV=ide1LA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-08 23:15 ` Or Gerlitz [not found] ` <CAJ3xEMgQNMtxgTKC0zaKgy-WGugf6KwT7Ys5h3_RbN_7qd2=tg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-15 19:03 ` device attr cleanup Doug Ledford [not found] ` <56706414.8010807-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-12-16 5:53 ` Or Gerlitz [not found] ` <5670FC5E.8070405-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 2015-12-16 13:40 ` Sagi Grimberg [not found] ` <567169B3.4040908-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2015-12-16 14:03 ` Or Gerlitz 2015-12-22 7:56 ` Or Gerlitz [not found] ` <CAJ3xEMiGOF79o1OSgxgk2tN+kY91uiNY5Aq+xu2U0Sur-htxpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2015-12-22 19:19 ` Doug Ledford [not found] ` <5679A254.2040809-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-12-22 20:37 ` Doug Ledford 2015-12-09 18:44 ` device attr cleanup (was: Handle mlx4 max_sge_rd correctly) Christoph Hellwig [not found] ` <20151209184448.GC4522-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-10 17:46 ` Doug Ledford [not found] ` <5669BA8E.30200-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-12-10 23:29 ` Christoph Hellwig [not found] ` <20151210232956.GA26009-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2015-12-11 4:56 ` Doug Ledford [not found] ` <566A5792.9080102-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-12-11 6:14 ` Jason Gunthorpe [not found] ` <20151211061433.GB16513-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2015-12-11 17:10 ` Doug Ledford
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.