Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Mon, Oct 21, 2024 at 12:26:54PM +1100, Alexey Kardashevskiy wrote: > On 18/10/24 02:37, Jason Gunthorpe wrote: > > On Thu, Oct 17, 2024 at 11:14:16AM -0300, Jason Gunthorpe wrote: > > > On Wed, Oct 09, 2024 at 09:38:02AM -0700, Nicolin Chen wrote: > > > > > > > @@ -217,12 +217,12 @@ iommufd_object_put_and_try_destroy(struct > > > > iommufd_ctx *ictx, > > > > iommufd_object_remove(ictx, obj, obj->id, 0); > > > > } > > > > > > > > -struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, > > > > -size_t size, > > > > -enum iommufd_object_type type); > > > > +struct iommufd_object *iommufd_object_alloc_elm(struct iommufd_ctx > > > > *ictx, > > > > + size_t size, > > > > + enum iommufd_object_type > > > > type); > > > > > > Maybe call it raw instead of elm? elm suggests it is an item in an > > > array or likewise > > > > Or keep this as the __ and rename > > > > #define __iommufd_object_alloc(ictx, ptr, type, obj) > >\ > > > > That one to _elm like this: > > > > #define iommufd_object_alloc_elm(ictx, ptr, type, elm) > > \ > > container_of(_iommufd_object_alloc( > > \ > >ictx, > > \ > >sizeof(*(ptr)) + BUILD_BUG_ON_ZERO( > > \ > > > > offsetof(typeof(*(ptr)), \ > > obj) != 0), > > \ > >type), > > \ > >typeof(*(ptr)), elm) > > > > #define iommufd_object_alloc(ictx, ptr, type) \ > > iommufd_object_alloc_elm(ictx, ptr, type, obj) > > > Bikeshedding, yay :) > > After starring at it for 10min - honestly - ditch > iommufd_object_alloc_elm() and just pass "obj" (or "common.obj" in that > single other occasion) to iommufd_object_alloc(). > > __iommufd_object_alloc() - a function - will the actual alloc, > iommufd_object_alloc() - a macro - will do the types + call the __ > variant, simple and no naming issues. All three-level helpers have callers. So that would be a bigger patch than I expected to include in this series. Maybe I should just drop this patch, since it's not functionally necessary. If we want to clean the whole thing, can do with a separate series. > And it would be real nice if it was "iobj" not this "obj" which is way > too generic. Thanks, Again, the renaming would be across the whole folder, not only here. So, I think it could be a separate cleanup series later. Thanks Nicolin
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On 18/10/24 02:37, Jason Gunthorpe wrote: On Thu, Oct 17, 2024 at 11:14:16AM -0300, Jason Gunthorpe wrote: On Wed, Oct 09, 2024 at 09:38:02AM -0700, Nicolin Chen wrote: @@ -217,12 +217,12 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx, iommufd_object_remove(ictx, obj, obj->id, 0); } -struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, -size_t size, -enum iommufd_object_type type); +struct iommufd_object *iommufd_object_alloc_elm(struct iommufd_ctx *ictx, + size_t size, + enum iommufd_object_type type); Maybe call it raw instead of elm? elm suggests it is an item in an array or likewise Or keep this as the __ and rename #define __iommufd_object_alloc(ictx, ptr, type, obj) \ That one to _elm like this: #define iommufd_object_alloc_elm(ictx, ptr, type, elm) \ container_of(_iommufd_object_alloc(\ ictx, \ sizeof(*(ptr)) + BUILD_BUG_ON_ZERO( \ offsetof(typeof(*(ptr)), \ obj) != 0), \ type),\ typeof(*(ptr)), elm) #define iommufd_object_alloc(ictx, ptr, type) \ iommufd_object_alloc_elm(ictx, ptr, type, obj) Bikeshedding, yay :) After starring at it for 10min - honestly - ditch iommufd_object_alloc_elm() and just pass "obj" (or "common.obj" in that single other occasion) to iommufd_object_alloc(). __iommufd_object_alloc() - a function - will the actual alloc, iommufd_object_alloc() - a macro - will do the types + call the __ variant, simple and no naming issues. And it would be real nice if it was "iobj" not this "obj" which is way too generic. Thanks, Then you can keep the pattern of _ being the allocation function of the macro Jason -- Alexey
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Thu, Oct 17, 2024 at 09:48:23AM -0700, Nicolin Chen wrote: > On Thu, Oct 17, 2024 at 01:35:07PM -0300, Jason Gunthorpe wrote: > > On Thu, Oct 17, 2024 at 09:12:03AM -0700, Nicolin Chen wrote: > > > > > > Then you can keep the pattern of _ being the allocation function of > > > > the macro > > > > > > If I get it correctly, the change would be > > > [From] > > > level-0: iommufd_object_alloc() > > > level-1: __iommufd_object_alloc() > > > level-2: _iommufd_object_alloc() > > > [To] > > > level-0: iommufd_object_alloc() > > > level-1: iommufd_object_alloc_elm() > > > level-2: _iommufd_object_alloc() > > > > > > i.e. change the level-1 only. > > > > You could also call it _iommufd_object_alloc_elm() to keep the pattern > > > > Maymbe "member" is a better word here than elm > > Ack. > > level-0: iommufd_object_alloc() > level-1: _iommufd_object_alloc_member() > level-2: _iommufd_object_alloc() Keep the pattern: level-0: iommufd_object_alloc() level-1: iommufd_object_alloc_member() level-2: _iommufd_object_alloc_member() Jason
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Thu, Oct 17, 2024 at 01:35:07PM -0300, Jason Gunthorpe wrote: > On Thu, Oct 17, 2024 at 09:12:03AM -0700, Nicolin Chen wrote: > > > > Then you can keep the pattern of _ being the allocation function of > > > the macro > > > > If I get it correctly, the change would be > > [From] > > level-0: iommufd_object_alloc() > > level-1: __iommufd_object_alloc() > > level-2: _iommufd_object_alloc() > > [To] > > level-0: iommufd_object_alloc() > > level-1: iommufd_object_alloc_elm() > > level-2: _iommufd_object_alloc() > > > > i.e. change the level-1 only. > > You could also call it _iommufd_object_alloc_elm() to keep the pattern > > Maymbe "member" is a better word here than elm Ack. level-0: iommufd_object_alloc() level-1: _iommufd_object_alloc_member() level-2: _iommufd_object_alloc() Thanks Nicolin
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Thu, Oct 17, 2024 at 09:12:03AM -0700, Nicolin Chen wrote: > > Then you can keep the pattern of _ being the allocation function of > > the macro > > If I get it correctly, the change would be > [From] > level-0: iommufd_object_alloc() > level-1: __iommufd_object_alloc() > level-2: _iommufd_object_alloc() > [To] > level-0: iommufd_object_alloc() > level-1: iommufd_object_alloc_elm() > level-2: _iommufd_object_alloc() > > i.e. change the level-1 only. You could also call it _iommufd_object_alloc_elm() to keep the pattern Maymbe "member" is a better word here than elm Jason
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Thu, Oct 17, 2024 at 12:37:49PM -0300, Jason Gunthorpe wrote: > On Thu, Oct 17, 2024 at 11:14:16AM -0300, Jason Gunthorpe wrote: > > On Wed, Oct 09, 2024 at 09:38:02AM -0700, Nicolin Chen wrote: > > > > > @@ -217,12 +217,12 @@ iommufd_object_put_and_try_destroy(struct > > > iommufd_ctx *ictx, > > > iommufd_object_remove(ictx, obj, obj->id, 0); > > > } > > > > > > -struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, > > > - size_t size, > > > - enum iommufd_object_type type); > > > +struct iommufd_object *iommufd_object_alloc_elm(struct iommufd_ctx *ictx, > > > + size_t size, > > > + enum iommufd_object_type type); > > > > Maybe call it raw instead of elm? elm suggests it is an item in an > > array or likewise > > Or keep this as the __ and rename You mean "_" v.s. "__"? > #define __iommufd_object_alloc(ictx, ptr, type, obj) > \ > > That one to _elm like this: > > #define iommufd_object_alloc_elm(ictx, ptr, type, elm) >\ > container_of(_iommufd_object_alloc(\ >ictx, \ >sizeof(*(ptr)) + BUILD_BUG_ON_ZERO( \ > offsetof(typeof(*(ptr)), \ > obj) != 0), \ >type),\ >typeof(*(ptr)), elm) > > #define iommufd_object_alloc(ictx, ptr, type) \ > iommufd_object_alloc_elm(ictx, ptr, type, obj) > > Then you can keep the pattern of _ being the allocation function of > the macro If I get it correctly, the change would be [From] level-0: iommufd_object_alloc() level-1: __iommufd_object_alloc() level-2: _iommufd_object_alloc() [To] level-0: iommufd_object_alloc() level-1: iommufd_object_alloc_elm() level-2: _iommufd_object_alloc() i.e. change the level-1 only. Thanks Nicolin
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Thu, Oct 17, 2024 at 11:14:16AM -0300, Jason Gunthorpe wrote: > On Wed, Oct 09, 2024 at 09:38:02AM -0700, Nicolin Chen wrote: > > > @@ -217,12 +217,12 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx > > *ictx, > > iommufd_object_remove(ictx, obj, obj->id, 0); > > } > > > > -struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, > > -size_t size, > > -enum iommufd_object_type type); > > +struct iommufd_object *iommufd_object_alloc_elm(struct iommufd_ctx *ictx, > > + size_t size, > > + enum iommufd_object_type type); > > Maybe call it raw instead of elm? elm suggests it is an item in an > array or likewise Or keep this as the __ and rename #define __iommufd_object_alloc(ictx, ptr, type, obj) \ That one to _elm like this: #define iommufd_object_alloc_elm(ictx, ptr, type, elm) \ container_of(_iommufd_object_alloc(\ ictx, \ sizeof(*(ptr)) + BUILD_BUG_ON_ZERO( \ offsetof(typeof(*(ptr)), \ obj) != 0), \ type),\ typeof(*(ptr)), elm) #define iommufd_object_alloc(ictx, ptr, type) \ iommufd_object_alloc_elm(ictx, ptr, type, obj) Then you can keep the pattern of _ being the allocation function of the macro Jason
Re: [PATCH v3 02/11] iommufd: Rename _iommufd_object_alloc to iommufd_object_alloc_elm
On Wed, Oct 09, 2024 at 09:38:02AM -0700, Nicolin Chen wrote: > @@ -217,12 +217,12 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx > *ictx, > iommufd_object_remove(ictx, obj, obj->id, 0); > } > > -struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, > - size_t size, > - enum iommufd_object_type type); > +struct iommufd_object *iommufd_object_alloc_elm(struct iommufd_ctx *ictx, > + size_t size, > + enum iommufd_object_type type); Maybe call it raw instead of elm? elm suggests it is an item in an array or likewise Naming aside Reviewed-by: Jason Gunthorpe Jason
