Skip to contentSkip to conversation

blk_iopoll_enabled

DISCUSSION Sagi Grimberg 2 participants 4 messages Mar 12, 2014 – Mar 13, 2014 SCSI

lore
Share & tools
lore
#0 lore raw

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.

#1 lore raw
Quoted 9 lines · Sagi Grimberg

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);

Quoted 6 lines

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.

Signature · 1 line
Jens Axboe
#2 lore raw
Quoted 17 lines · Jens Axboe
Quoted 9 lines · Sagi Grimberg

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);

Quoted 6 lines

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?

Quoted 6 lines

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.

Signature · 1 line
Jens Axboe
#3 lore raw
Quoted 18 lines · Jens Axboe
Quoted 17 lines · Jens Axboe
Quoted 15 lines · Sagi Grimberg

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?

Quoted 9 lines
Quoted 6 lines

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.