RE: [PATCH 0/5] Feature enhancements for ses module

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

 



Hi Doug, 

I agree what it is difficult to handle all elements, and thus using sg_ses probably makes more sense. However, it helps to handle some HDD related fields in the kernel, as the kernel can generate mapping between a device to the SES device element (or array device element): 

/sys/block/sdc/device/enclosure_deviceXXX/

With patch 5, we can easily power off a running HDD by 

echo off > /sys/block/sdc/device/enclosure_deviceXXX/power_status

This is very useful for systems like Cold Storage, where HDDs are being powered on/off frequently. 


In current code, ses_set_page2_descriptor already clear reserved field for all elements, and only send non-zero for the device element or array device. Patch 5 also handles reserved field of these two elements in init_device_slot_control:

dest_desc[2] &= 0xde;
dest_desc[3] &= 0x3c;

So I think we can control fault, locate, active, and power_status of each HDD without issue. 

Please let me know your suggestion on this. 

Thanks,
Song

> -----Original Message-----
> From: Song Liu
> Sent: Wednesday, October 22, 2014 8:42 PM
> To: 'dgilbert@xxxxxxxxxxxx'; Jens Axboe; linux-scsi@xxxxxxxxxxxxxxx
> Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig
> Subject: RE: [PATCH 0/5] Feature enhancements for ses module
> 
> Hi Doug,
> 
> The power on/off field together with "fault", "locate", and "active" are for HDD
> (i.e. DeviceSlot and ArrayDeviceSlot). They are not for enclosure or other
> elements. So we are not dealing with power off duration, etc. here.
> 
> Thanks,
> Song
> 
> > -----Original Message-----
> > From: Douglas Gilbert [mailto:dgilbert@xxxxxxxxxxxx]
> > Sent: Wednesday, October 22, 2014 6:17 PM
> > To: Song Liu; Jens Axboe; linux-scsi@xxxxxxxxxxxxxxx
> > Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig
> > Subject: Re: [PATCH 0/5] Feature enhancements for ses module
> >
> > On 14-10-23 01:01 AM, Song Liu wrote:
> > > Hi Doug,
> > >
> > > I am not sure whether I fully understand the scenario. Actually
> > > patch 5 tries to
> > clear all reserved bits:
> > >
> > > + dest_desc[0] = 0;
> > > + /* only clear byte 1 for ENCLOSURE_COMPONENT_DEVICE */ if
> > > + (ecomp->type == ENCLOSURE_COMPONENT_DEVICE)
> > > +   dest_desc[1] = 0;
> > > + dest_desc[2] &= 0xde;
> > > + dest_desc[3] &= 0x3c;
> > >
> > > Would this work for device that rejects request with 1 in RESERVED areas?
> >
> >
> > That is a pretty asymmetric element type, assuming we are talking
> > about the "enclosure control" and "enclosure status" elements (i.e.
> > etc=0xe). My guess would be:
> >
> >      dest_desc[0] = 0x80 | (src_desc[0] & 40);
> >      dest_desc[1] = 0x80 & src_desc[1];
> >      dest_desc[2] = (pc_req << 6) | pc_delay;
> >      dest_desc[3] = 0xff & src_desc[3]; or if you have a new
> > power_off_duration:
> >      dest_desc[3] = (power_off_duration << 2) | (src_desc[3] & 0x3);
> >
> > In byte 0 the top bit (SELECT) must be set or the enclosure will
> > ignore any other settings in that element. If the PRDFAIL bit is
> > already set, then that setting will be maintained.
> > SES-3 has a note about clearing DISABLE and SWAP.
> >
> > In byte 1 is if the identifier (LED ?) is active (saying blinking)
> > prior to this power cycle request, then it will stay blinking until
> > the power drops. If the enclosure was really clever it might keep
> > blinking after the power cycle :-)
> >
> > Also notice that the requested power cycle can be cancelled up to the
> > "time until power cycle" drops to zero.
> >
> > >> -----Original Message-----
> > >> From: Douglas Gilbert [mailto:dgilbert@xxxxxxxxxxxx]
> > >> Sent: Wednesday, October 22, 2014 3:29 PM
> > >> To: Jens Axboe; Song Liu; linux-scsi@xxxxxxxxxxxxxxx
> > >> Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig
> > >> Subject: Re: [PATCH 0/5] Feature enhancements for ses module
> > >>
> > >> On 14-10-22 09:12 PM, Jens Axboe wrote:
> > >>> On 08/25/2014 11:34 AM, Song Liu wrote:
> > >>>> From: Song Liu [mailto:songliubraving@xxxxxx]
> > >>>> Sent: Monday, August 25, 2014 10:26 AM
> > >>>> To: Song Liu
> > >>>> Subject: [PATCH 0/5] Feature enhancements for ses module
> > >>>>
> > >>>> These patches include a few enhancements to ses module:
> > >>>>
> > >>>> 1: close potential race condition by at enclosure race condition
> > >>>>
> > >>>> 2,3,4: add enclosure id and device slot, so we can create symlink
> > >>>>          in /dev/disk/by-slot:
> > >>>>     # ls -d /dev/disk/by-slot/*
> > >>>>       /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0
> > >>>>
> > >>>> 5: add ability to power on/off device with power_status file in
> > >>>>      sysfs.
> > >>
> > >> Had a rude awakening with sg_ses recently when setting a field in
> > >> the enclosure control dpage. That is what is being done in point 5: above.
> > >>
> > >> The time honoured technique is to read the corresponding enclosure
> > >> status dpage, find the correct element, twiddle the field of
> > >> interest, set the SELECT bit and write it back. The idea is
> > >> maintain any other
> > field settings in that element.
> > >> And this is what the ses module does.
> > >>
> > >> There is at least one SES device out there that rejects the write
> > >> if there are bits set in RESERVED locations. According to SPC-4 a
> > >> device may do that. Look at the status (read) and control (write)
> > >> variants of each element type: many times the control variant has less
> fields.
> > >>
> > >> To fix that the read-modify-write cycle needs to be changed to a
> > >> read-mask- modify-write cycle where the "mask" is the allowable
> > >> write mask (there would be one for each element type).
> > >>
> > >> This is ugly and may create problems in the future if and when
> > >> T10 adds a new field in an element. About that time T10 should
> > >> think about refining the meaning of RESERVED in SES to outlaw SES
> > >> devices creating this particular time waster.
> > >>
> > >> Doug Gilbert
> > >>
> > >>>> Dan Williams (4):
> > >>>>     SES: close potential registration race
> > >>>>     SES: generate KOBJ_CHANGE on enclosure attach
> > >>>>     SES: add enclosure logical id
> > >>>>     SES: add reliable slot attribute
> > >>>>
> > >>>> Song Liu (1):
> > >>>>     SES: Add power_status to SES enclosure component
> > >>>
> > >>> Guys, where are we with these? Seemed like they got reviewed (and
> > >>> updates/fixes posted), then nothing else happened. Would be nice
> > >>> to get these upstream, so we don't have to carry them at FB.
> > >>>
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo
> > > info at
> > > https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/ma
> > > jo
> > > rdomo-
> >
> info.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=J3jGe56dIPfS5TN6DM
> > >
> >
> 82UYbbeR1j2viaiSJI40tv6lE%3D%0A&m=CIfJIr9ka37ZOOTBDMcaf3cmkg1%2BV
> > Rv4oz
> > >
> >
> 0zbf%2Fb24o%3D%0A&s=792fd953749b38a8e8181e2f36a2b9102ae9a3096f85f
> > 95690
> > > 4aa8201dde45d6
> > >

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux