Re: for-4.12/block branch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/21/2017 11:07 AM, Jens Axboe wrote:
> On 04/21/2017 11:05 AM, Bart Van Assche wrote:
>> On Fri, 2017-04-21 at 10:56 -0600, Jens Axboe wrote:
>>> On 04/21/2017 10:48 AM, Jens Axboe wrote:
>>>> I wonder if it's an imbalance in the preempt count. Looking at it, it
>>>> looks like we're not clearing the alloc data. But I would think that
>>>> would potentially cause much worse problems, but maybe we got lucky?
>>>>
>>>> Let me generate a cleanup patch for that.
>>>
>>> Something like the below.
>>> [ ... ]
>>> +static inline void blk_mq_init_alloc_data(struct blk_mq_alloc_data *data,
>>> +					  unsigned int flags)
>>> +{
>>> +	data->q = NULL;
>>> +	data->flags = flags;
>>> +	data->shallow_depth = 0;
>>> +	data->ctx = NULL;
>>> +	data->hctx = NULL;
>>> +}
>>
>> Hello Jens,
>>
>> Maybe I'm overlooking something but I don't see how this patch can make
>> a difference since the compiler zero-initializes struct members that have
>> not been mentioned explicitly as designated initializers? A common way
>> to zero-initialize a struct is as follows:
>>
>> struct <struct_name> <variable_name> = { };
> 
> Maybe my memory is bad, but we're explicitly setting flags, but not doing
> a zero fill after that.
> 
> 	struct foo foo { .member = bla };
> 
> vs
> 
> 	struct foo foo { .member = bla, };
> 
> But you must be right, because this would barf all over the place with stack
> garbage otherwise. I'll double check I get the same generated code here...

It is of course me smoking crack. I think my recollection dates back to some
bug gcc had in that area, either with suboptimal fill/init, or just not doing
it. In any case, the current code is fine, which is good.

-- 
Jens Axboe




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux