Re: device attr cleanup

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- 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 --]