* blk_iopoll_enabled
@ 2014-03-12 17:35 Sagi Grimberg
2014-03-13 3:40 ` blk_iopoll_enabled Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: Sagi Grimberg @ 2014-03-12 17:35 UTC (permalink / raw)
To: Jens Axboe; +Cc: Bart Van Assche, Or Gerlitz, Oren Duer, linux-scsi
Hey Jens,
So I'm looking at blk_iopoll for iSER & SRP and I have one gap I would
like to get your input on.
In blk-iopoll.c I see:
int blk_iopoll_enabled = 1;
EXPORT_SYMBOL(blk_iopoll_enabled);
This is set to 1 and exported for everyone to condition and *modify* it.
The documentation says the user must check this when using blk_iopoll.
My question is why? it should always be true shouldn't it?
Moreover, isn't it better to protect it using a get() function of it?
can't someone set it to 0 and shutdown the lights on everyone else?
Thanks,
Sagi.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: blk_iopoll_enabled
2014-03-12 17:35 blk_iopoll_enabled Sagi Grimberg
@ 2014-03-13 3:40 ` Jens Axboe
2014-03-13 15:34 ` blk_iopoll_enabled Jens Axboe
0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2014-03-13 3:40 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Bart Van Assche, Or Gerlitz, Oren Duer, linux-scsi
On 2014-03-12 11:35, Sagi Grimberg wrote:
> Hey Jens,
>
> So I'm looking at blk_iopoll for iSER & SRP and I have one gap I would
> like to get your input on.
>
> In blk-iopoll.c I see:
> int blk_iopoll_enabled = 1;
> EXPORT_SYMBOL(blk_iopoll_enabled);
>
> This is set to 1 and exported for everyone to condition and *modify* it.
> The documentation says the user must check this when using blk_iopoll.
>
> My question is why? it should always be true shouldn't it?
> Moreover, isn't it better to protect it using a get() function of it?
> can't someone set it to 0 and shutdown the lights on everyone else?
We can kill it. IIRC, it was mostly used during development to test
functionality and performance (and wired to a sysfs toggle file).
But... Just because it's exported doesn't mean that random users are
allowed to just disable it. What's left now is an always-on, so we can
remove it.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: blk_iopoll_enabled
2014-03-13 3:40 ` blk_iopoll_enabled Jens Axboe
@ 2014-03-13 15:34 ` Jens Axboe
2014-03-13 18:48 ` blk_iopoll_enabled Sagi Grimberg
0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2014-03-13 15:34 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Bart Van Assche, Or Gerlitz, Oren Duer, linux-scsi
On 03/12/2014 09:40 PM, Jens Axboe wrote:
> On 2014-03-12 11:35, Sagi Grimberg wrote:
>> Hey Jens,
>>
>> So I'm looking at blk_iopoll for iSER & SRP and I have one gap I would
>> like to get your input on.
>>
>> In blk-iopoll.c I see:
>> int blk_iopoll_enabled = 1;
>> EXPORT_SYMBOL(blk_iopoll_enabled);
>>
>> This is set to 1 and exported for everyone to condition and *modify* it.
>> The documentation says the user must check this when using blk_iopoll.
>>
>> My question is why? it should always be true shouldn't it?
>> Moreover, isn't it better to protect it using a get() function of it?
>> can't someone set it to 0 and shutdown the lights on everyone else?
>
> We can kill it. IIRC, it was mostly used during development to test
> functionality and performance (and wired to a sysfs toggle file).
>
> But... Just because it's exported doesn't mean that random users are
> allowed to just disable it. What's left now is an always-on, so we can
> remove it.
It _is_ still exported, I just forgot it was put in sysctl. I'll kill it
off completely now. It's safe to toggle from user space, but only
practically so if IO is idle. It makes no attempt to ensure that, as it
was just a debug thing.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: blk_iopoll_enabled
2014-03-13 15:34 ` blk_iopoll_enabled Jens Axboe
@ 2014-03-13 18:48 ` Sagi Grimberg
0 siblings, 0 replies; 4+ messages in thread
From: Sagi Grimberg @ 2014-03-13 18:48 UTC (permalink / raw)
To: Jens Axboe; +Cc: Bart Van Assche, Or Gerlitz, Oren Duer, linux-scsi
On 3/13/2014 5:34 PM, Jens Axboe wrote:
> On 03/12/2014 09:40 PM, Jens Axboe wrote:
>> On 2014-03-12 11:35, Sagi Grimberg wrote:
>>> Hey Jens,
>>>
>>> So I'm looking at blk_iopoll for iSER & SRP and I have one gap I would
>>> like to get your input on.
>>>
>>> In blk-iopoll.c I see:
>>> int blk_iopoll_enabled = 1;
>>> EXPORT_SYMBOL(blk_iopoll_enabled);
>>>
>>> This is set to 1 and exported for everyone to condition and *modify*
>>> it.
>>> The documentation says the user must check this when using blk_iopoll.
>>>
>>> My question is why? it should always be true shouldn't it?
>>> Moreover, isn't it better to protect it using a get() function of it?
>>> can't someone set it to 0 and shutdown the lights on everyone else?
>>
>> We can kill it. IIRC, it was mostly used during development to test
>> functionality and performance (and wired to a sysfs toggle file).
>>
>> But... Just because it's exported doesn't mean that random users are
>> allowed to just disable it. What's left now is an always-on, so we can
>> remove it.
>
> It _is_ still exported, I just forgot it was put in sysctl. I'll kill
> it off completely now. It's safe to toggle from user space, but only
> practically so if IO is idle. It makes no attempt to ensure that, as
> it was just a debug thing.
>
Thanks
Sagi.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-13 18:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-12 17:35 blk_iopoll_enabled Sagi Grimberg
2014-03-13 3:40 ` blk_iopoll_enabled Jens Axboe
2014-03-13 15:34 ` blk_iopoll_enabled Jens Axboe
2014-03-13 18:48 ` blk_iopoll_enabled Sagi Grimberg
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.