SCSI sg: Questions regarding jiffies usage

SCSI sg: Questions regarding jiffies usage

[Adrian Bunk]

Hi,

I have some questions regarding jiffies usage in the SCSI sg driver in 
2.6.7-mm7:


Is there a good reason for the local sg_ms_to_jif/sg_jif_to_ms functions  
instead of using the time.h msecs_to_jiffies/jiffies_to_msecs macros?

In function sg_new_write:
   timeout = sg_ms_to_jif(srp->header.timeout);
How is it possible that srp->header.timeout isn't already
in jiffies?

Why do both sg_add_request and sg_common_write set
  duration = jiffies
?


TIA
Adrian

BTW: Please Cc me on replies.

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: SCSI sg: Questions regarding jiffies usage

[Douglas Gilbert]

Adrian Bunk wrote:
> Hi,
> 
> I have some questions regarding jiffies usage in the SCSI sg driver in 
> 2.6.7-mm7:
> 
> 
> Is there a good reason for the local sg_ms_to_jif/sg_jif_to_ms functions  
> instead of using the time.h msecs_to_jiffies/jiffies_to_msecs macros?

Adrian,
I don't think the msec_to_jiffies() macro was there when I
wanted it. So the sg driver should be changed to use the
macros in time.h .

> In function sg_new_write:
>    timeout = sg_ms_to_jif(srp->header.timeout);
> How is it possible that srp->header.timeout isn't already
> in jiffies?

The sg_new_write() function is invoked when the sg_io_hdr
interface structure is used (compared with the older
sg_header structure). The unit of the timeout in the
sg_io_hdr structure is milliseconds [which is probably
overkill]. The mid level wants timeouts in jiffies.

> Why do both sg_add_request and sg_common_write set
>   duration = jiffies

It doesn't look like both are required.

Doug Gilbert

Re: SCSI sg: Questions regarding jiffies usage

[Adrian Bunk]

On Mon, Jul 12, 2004 at 03:35:06PM -0400, Douglas Gilbert wrote:
> Adrian Bunk wrote:
> >Hi,
> >
> >I have some questions regarding jiffies usage in the SCSI sg driver in 
> >2.6.7-mm7:
> >
> >
> >Is there a good reason for the local sg_ms_to_jif/sg_jif_to_ms functions  
> >instead of using the time.h msecs_to_jiffies/jiffies_to_msecs macros?
> 
> Adrian,

Hi Doug,

> I don't think the msec_to_jiffies() macro was there when I
> wanted it. So the sg driver should be changed to use the
> macros in time.h .

I'll send a patch.

> >In function sg_new_write:
> >   timeout = sg_ms_to_jif(srp->header.timeout);
> >How is it possible that srp->header.timeout isn't already
> >in jiffies?
> 
> The sg_new_write() function is invoked when the sg_io_hdr
> interface structure is used (compared with the older
> sg_header structure). The unit of the timeout in the
> sg_io_hdr structure is milliseconds [which is probably
> overkill]. The mid level wants timeouts in jiffies.

As far as I can see, timeout is set in two places:

sg_add_sfp:
  sfp->timeout = SG_DEFAULT_TIMEOUT;

sg_ioctl:
  sfp->timeout = MULDIV (val, HZ, USER_HZ);

Isn't timeout set to jiffies in both places?

> >Why do both sg_add_request and sg_common_write set
> >  duration = jiffies
> 
> It doesn't look like both are required.
> 
> Doug Gilbert

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: SCSI sg: Questions regarding jiffies usage

[Douglas Gilbert]

Adrian Bunk wrote:
> On Mon, Jul 12, 2004 at 03:35:06PM -0400, Douglas Gilbert wrote:
> 
<snip/>

>>>In function sg_new_write:
>>>  timeout = sg_ms_to_jif(srp->header.timeout);
>>>How is it possible that srp->header.timeout isn't already
>>>in jiffies?
>>
>>The sg_new_write() function is invoked when the sg_io_hdr
>>interface structure is used (compared with the older
>>sg_header structure). The unit of the timeout in the
>>sg_io_hdr structure is milliseconds [which is probably
>>overkill]. The mid level wants timeouts in jiffies.
> 
> 
> As far as I can see, timeout is set in two places:
> 
> sg_add_sfp:
>   sfp->timeout = SG_DEFAULT_TIMEOUT;
> 
> sg_ioctl:
>   sfp->timeout = MULDIV (val, HZ, USER_HZ);
> 
> Isn't timeout set to jiffies in both places?
> 
Adrian,
There may be some confusion here. The older struct
sg_header interface did not have explicit per command timeouts.
As a poor substitute an ioctl was added to associate
a timeout (in seconds) to the file descriptor. This way
subsequent commands issued via that file descriptor
got the new timeout value. This is the "sfp->timeout"
value. The snippets above show the "sfp->timeout"
value being set to a default when a sg device is opened
and then being changed after a SG_SET_TIMEOUT ioctl call.

The newer sg_io_hdr interface has per command timeouts
(in milliseconds). A sg_io_hdr::timeout value overrides
the "sfp->timeout" value. In the above code the
sg_io_hdr::timeout value is in "srp->header.timeout".

Doug Gilbert

Re: SCSI sg: Questions regarding jiffies usage

[Adrian Bunk]

On Mon, Jul 12, 2004 at 07:11:54PM -0400, Douglas Gilbert wrote:

> Adrian,

Hi Doug,

> There may be some confusion here. The older struct
> sg_header interface did not have explicit per command timeouts.
> As a poor substitute an ioctl was added to associate
> a timeout (in seconds) to the file descriptor. This way
> subsequent commands issued via that file descriptor
> got the new timeout value. This is the "sfp->timeout"
> value. The snippets above show the "sfp->timeout"
> value being set to a default when a sg device is opened
> and then being changed after a SG_SET_TIMEOUT ioctl call.
> 
> The newer sg_io_hdr interface has per command timeouts
> (in milliseconds). A sg_io_hdr::timeout value overrides
> the "sfp->timeout" value. In the above code the
> sg_io_hdr::timeout value is in "srp->header.timeout".

thanks for the explanations.

> Doug Gilbert

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed