Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: MD component device renaming with udev and MD on full disk
From: NeilBrown @ 2014-11-25  2:55 UTC (permalink / raw)
  To: Jason Keltz; +Cc: linux-raid
In-Reply-To: <546FB1B7.3050304@cse.yorku.ca>

[-- Attachment #1: Type: text/plain, Size: 3467 bytes --]

On Fri, 21 Nov 2014 16:42:15 -0500 Jason Keltz <jas@cse.yorku.ca> wrote:

> Hi.
> 
> I have two questions about MD..
> 
> 1) I've written a udev rule to remap /dev/sdX devices on my system to 
> /dev/cXeYsZpA (controller, enclosure, slot, partition) mapping. When I 
> reboot the system, I see that all the devices in /dev are appropriately 
> renamed.  If I look at /proc/mdstat, it still has the kernel names of 
> the devices (/dev/sdX) even though those devices no longer exist.  If I 
> do an mdadm --detail /dev/mdX the system reports the proper device name 
> makeup.  I manually failed a device, and I got the correct device name 
> in the email.  I'm just wondering what command I would execute to make 
> /proc/mdstat update the devices in its output?

Names in /dev/ are link symlinks.  They point to the device (which is
identified by a pair of numbers: e.g. 8,1), but they aren't the device itself.

block device 8,1 is always "sda1" to the kernel, whether you access it through
a block-special file called "/dev/sda1" or "/dev/box-of-bits".

The names reported by /proc/mdstat are the kernel's internal names for the
devices.  You cannot get it to use the names that you have created in /dev.

"mdadm -D" is the best way to describe an array in terms of names that you
have chosen.


> 
> 2) Unrelated to 1) -- the argument re: using MD on full devices versus 
> partitions has been around for a long time.  I've been experimenting 
> with using it on full devices.  One of the arguments that I've read for 
> not using full devices is that apparently, if you have two devices that 
> are identical, but one of them is slightly smaller than the other due to 
> say, bad sectors, then these disks can't be used together in one MD 
> because they are different sizes. I'm wondering how valid that argument 
> is?  Surely it would make sense if MD was using full devices for it to 
> actually stop short of the end of the disk for situations like these...

There is no validity at all in that argument.
md is quite happy using devices of different sizes and will ignore extra
space provided on the larger device.

If you had an array with two devices of size X, and got a new device of size
"X - delta", then it is true that the new device cannot be used in the array.
I think I have read a suggestion of creating partitions smaller than the
device so that when the "X - delta" device is used, it is still big enough
for that smaller partition.

There are two reasons why this is an invalid argument for using partitions:
1/ modern drives have standardised sizes.  If you have a size-X device, then
   you will not find a size-"X - delta" device unless "delta" is very big.

2/ you can use the --size option of mdadm to make the array use less than
   the total space on the device to allow for later reconfigurations.

There are other valid  reasons for preferring whole devices or partitions,
such as some tool or other might get confused by one of the arrangements
(e.g. lilo or and installer).  And some people might be more comfortable with
one than the other.
But the size issue is completely irrelevant.

NeilBrown


> 
> Thanks for any help you can provide.
> 
> Jason.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: MD RAID5 hang on make_request
From: NeilBrown @ 2014-11-25  2:39 UTC (permalink / raw)
  To: Henry Cai; +Cc: linux-raid
In-Reply-To: <CANH1pmsL4dHvsmkj52=jpWGQmx5B39hY3w480pkJs7ZmKnpptg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

On Sun, 23 Nov 2014 09:08:55 +0800 Henry Cai <henryplusplus@gmail.com> wrote:

> With kernel-3.10, when raid5 is doing resync, unplug one of the
> raid5's disk, RAID5 will hang on raid5's make_request function, and
> cat /proc/mdstat show the RAID "resync=PENDING", mdadm version is
> 3.3.2. The platform is arm.
> 
> Any similar problems?

Not that I know of...

Can you test a more recent kernel and see if the problem persists?

Can you provide more specifics than "the make_request function".
e.g. a stack trace (from /proc/PID/stack), or similar.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: md raid performance with 3-18-rc3
From: NeilBrown @ 2014-11-25  2:37 UTC (permalink / raw)
  To: Manish Awasthi; +Cc: linux-raid
In-Reply-To: <5472E7DE.5070702@caviumnetworks.com>

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

On Mon, 24 Nov 2014 13:40:06 +0530 Manish Awasthi
<manish.awasthi@caviumnetworks.com> wrote:

> Hi,
> 
> We benchmarked the md raid driver performance on 3-18-rc3 kernel and 
> compared the results with that of 3.6.11. The reason for this exercise 
> is to understand if multithreaded raid driver has any performance 
> benefits over 3.6.11 which is single threaded. Here are some details 
> about the setup

Thanks for doing this!!!! I love it when people report test results.


> 
> System: Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz 4 cores (8threads), 
> 8GB RAM.
> Setup: 3 SSDs create a raid5 array
> test tool: iozone (only read/re-read, write/re-write tested), blocksize: 
> 4k-64k, filesize: 1Gig to 200Gig
> 
> Comparison was done for speed of data transfer in kBytes/sec and also 
> the CPU utilization as reported by iozone.
> 
> raid on 3.18.0-rc3 performed much worse than raid on 3.6.11.
> 
> Read/Write: raid on 3.18.0-rc3 operated at almost half the speed of raid 
> on 3.6.11

That really isn't very good.... Can you try some of the kernels in between
and see if there was a single point where performance dropped, or if there
were several steps?


> 
> CPU Utilization: With md raid on 3.18.0-rc3, the CPU utilization was 
> less than half of md raid on 3.6.11 on WRITE operations. However, for 
> READ operations, 3.18-0.rc3 had more CPU utilization than 3.6.11.

Can you use "perf" to determine where the extra time is going?

  perf record
  run test
  stop perf
  perf report

or something like that.

> 
> Also, I noticed that scaling up the CPU cores of the system scales down 
> the raid througput with 3.18.0-rc3.

This is by writing numbers to "group_thread_cnt" ??? Can you provide a simple
table comparing thread count to throughput?  Or maybe a graph.  I love
graphs :-)


> 
> I do have detailed logs of the comparison but I'm not sure I should send 
> those on this mailing list.

A few megabytes?  Yes.  100Meg?  No.

If you could put them on a website somewhere that I can browse or download
I'll try to have a look.

> 
> If my observation aligns with someone else's, then what is really the 
> gain with multithreaded raid.

Some testing shows real improvements.  Obviously we cannot test everything
and I'm very glad to have extra testing from other people.
If we can quantify the regressions and confirm exactly when they occurred, we
can start looking for a solution.

Thanks a lot!

NeilBrown


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: raid10 - won't rebuild - assigns all added disks as spares
From: NeilBrown @ 2014-11-25  2:28 UTC (permalink / raw)
  To: Jonathan Molyneux; +Cc: linux RAID
In-Reply-To: <5473E018.3020507@infinitedepth.com.au>

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

On Tue, 25 Nov 2014 12:49:12 +1100 Jonathan Molyneux
<jonathan@infinitedepth.com.au> wrote:

> Hi Everyone,
> 
> Have a strange situation that hasn't happened before.
> Running Debian 7.7 with kernel version 3.2.63-2+deb7u1.
> Have a raid10 that runs the server (boot's off a raid1) that after 
> replacing a failed disk, just won't rebuild.
> 
> This is what it looks like without the disk (failed & removed):
> md1 : active raid10 sda2[6] sdc2[4] sdb2[1]
>        1952987136 blocks super 1.2 512K chunks 2 far-copies [4/3] [UUU_]
>        bitmap: 8/15 pages [32KB], 65536KB chunk
> 
> Then when the disk is added:
> md1 : active raid10 sdd2[5](S) sda2[6] sdc2[4] sdb2[1]
>        1952987136 blocks super 1.2 512K chunks 2 far-copies [4/3] [UUU_]
>        bitmap: 8/15 pages [32KB], 65536KB chunk
> 
> Nothing unusual is being spat out in dmesg.
> When removing the disk:
> [313434.073997] md: unbind<sdd2>
> [313434.138307] md: export_rdev(sdd2)
> When adding the disk:
> [313468.056484] md: bind<sdd2>
> 
> This is a strange one that I haven't had before.
> Any thoughts on how to kick the rebuild off without needing a reboot ?

I'm sure I've seen this bug before... and fixed it.
I don't remember the details and cannot find anything obvious in change logs.

You could try

   echo recover > /sys/block/md1/md/sync_action

Alternately, if you are re-adding a disk that had just been removed, you could

   mdadm /dev/md1 --remove /dev/sdd2
   mdadm --zero /dev/sdd2
   mdadm /dev/md1 --add /dev/sdd2

that will force a full recovery instead of just a bitmap-based recovery.
That will of course take longer than a bitmap-based recover, but seeing the
bitmap based recovery isn't starting, that could still be an improvement.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* raid10 - won't rebuild - assigns all added disks as spares
From: Jonathan Molyneux @ 2014-11-25  1:49 UTC (permalink / raw)
  To: linux RAID

Hi Everyone,

Have a strange situation that hasn't happened before.
Running Debian 7.7 with kernel version 3.2.63-2+deb7u1.
Have a raid10 that runs the server (boot's off a raid1) that after 
replacing a failed disk, just won't rebuild.

This is what it looks like without the disk (failed & removed):
md1 : active raid10 sda2[6] sdc2[4] sdb2[1]
       1952987136 blocks super 1.2 512K chunks 2 far-copies [4/3] [UUU_]
       bitmap: 8/15 pages [32KB], 65536KB chunk

Then when the disk is added:
md1 : active raid10 sdd2[5](S) sda2[6] sdc2[4] sdb2[1]
       1952987136 blocks super 1.2 512K chunks 2 far-copies [4/3] [UUU_]
       bitmap: 8/15 pages [32KB], 65536KB chunk

Nothing unusual is being spat out in dmesg.
When removing the disk:
[313434.073997] md: unbind<sdd2>
[313434.138307] md: export_rdev(sdd2)
When adding the disk:
[313468.056484] md: bind<sdd2>

This is a strange one that I haven't had before.
Any thoughts on how to kick the rebuild off without needing a reboot ?

PS

Rebooting the server is an option, just would require some scheduling.

^ permalink raw reply

* Re: [PATCH 1/5] imsm: support for OROMs shared by multiple HBAs
From: NeilBrown @ 2014-11-25  0:51 UTC (permalink / raw)
  To: Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak
In-Reply-To: <546E29CB.1010408@intel.com>

[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]

On Thu, 20 Nov 2014 18:50:03 +0100 Artur Paszkiewicz
<artur.paszkiewicz@intel.com> wrote:

> On 11/20/2014 04:07 AM, NeilBrown wrote:
> > On Wed, 19 Nov 2014 13:53:26 +0100 Artur Paszkiewicz
> > <artur.paszkiewicz@intel.com> wrote:
> > 
> >> HBAs can share OROMs (e.g. SATA/sSATA). They are matched by PCI device
> >> id. Removed populated_orom/efi and imsm_orom/efi arrays - they are
> >> replaced by oroms array and functions get_orom_by_device_id(),
> >> add_orom(), add_orom_device_id().
> >>
> >> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> > 
> > Hi,
> >  this patch seems to make a lot more changes that the above brief description
> >  seems to suggest.
> >  Is there any chance of breaking it up into two or three parts, or at least
> >  describing everything that is being changed.
> > 
> >  I'm half tempted to just accept it as it is, as it is just "your" code that
> >  that is being changed, but I'd like to understand it if I can.
> > 
> > Thanks,
> > NeilBrown
> > 
> 
> Hi Neil,
> 
> Splitting this up reasonably turned out to be more difficult than I
> thought, so I'll try to provide a more detailed description of the
> changes. 
> 
> The IMSM platform code was based on an assumption that the OROM or UEFI
> capability structure (represented by struct imsm_orom) always belongs to
> only one HBA. This assumption is no longer valid, because of newer
> platforms with dual AHCI HBAs. Each HBA can have a separate OROM, but
> some versions have a combined OROM for both HBAs.
> 
> This patch implements this HBA-OROM relationship in struct orom_entry,
> which matches an OROM with a list of HBA PCI ids. All the detected
> orom_entries are stored and retrieved using a global array and the
> functions add_orom(), add_orom_device_id() and get_orom_by_device_id().
> This replaces the arrays: imsm_orom, populated_orom, imsm_efi,
> populated_efi.
> 
> The scan() function is extended to find all HBAs for an OROM. The list
> of their device ids is retrieved from the PCI Expansion ROM Data
> Structure, hence the additional field devListOffset in struct
> pciExpDataStructFormat.
> 
> In UEFI mode we can't read the PCI Expansion ROM Data Structure and the
> imsm_orom structures are stored in UEFI variables. They do not provide a
> similar device id list, so we also check the HBA PCI class to make sure
> that the HBA has RAID mode enabled.
> 
> In super-intel.c there are changes which allow spanning of IMSM
> containers over HBAs of the same type, but only if the HBAs share the
> same OROM.  This is done by comparing imsm_orom pointers, which (outside
> of platform-intel.c) always point to the global array containing all the
> detected oroms. Additional warnings are added to
> validate_container_imsm() to warn about potentially dangerous operations
> in all the possible cases, e.g. when an array is assembled using disks
> attached to HBAs with separate OROMs.
> 
> I hope you find this description helpful and that it will make the
> changes easier to understand.
> 

Much better - thanks!

I've applied all 5 patches - using v2 of patch 4, and this comment for patch
1.

Thanks,
NeilBrown


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: mdadm creates corrupt superblock
From: NeilBrown @ 2014-11-25  0:12 UTC (permalink / raw)
  To: Hans Kraus; +Cc: Linux-RAID
In-Reply-To: <546E0FC2.9090606@hanswkraus.com>

[-- Attachment #1: Type: text/plain, Size: 11711 bytes --]

On Thu, 20 Nov 2014 16:58:58 +0100 Hans Kraus <hans@hanswkraus.com> wrote:

> Hi Neil,
> 
> the partition table (output of 'sfdisk -d /dev/sdh'):
> 
> # partition table of /dev/sdh
> unit: sectors
> 
> /dev/sdh1 : start=     2048, size=1953523120, Id=83
> /dev/sdh2 : start=        0, size=        0, Id= 0
> /dev/sdh3 : start=        0, size=        0, Id= 0
> /dev/sdh4 : start=        0, size=        0, Id= 0
> 
> Makes that any sense to you?

This tells me that sdh1 starts at sector 2048 in sdh.
So sector 64 of sdh is not in any partition.  So if sector 64 of sdh is
faulty, that will not affect an array in sdh1....

But you mentioned sdb before ... did the names change (I know that does
happen).

How did you run "badblocks" and what exactly was the output?

There must be something we are missing here.

NeilBrown


> 
> Regards, Hans
> 
> On 11/19/2014 10:47 PM, NeilBrown wrote:
> > On Wed, 19 Nov 2014 10:55:05 +0100 Hans Kraus <hans@hanswkraus.com> wrote:
> >
> >> Hi,
> >>
> >> I think I found the problem. 'badblocks' reports sector 64 of the HD as
> >> errornous. But the HD neither reports a read error nor remaps that
> >> sector.
> >>
> >> Is it possible to parametrize mdadm that it doesn't use that sector?
> >
> > I'm surprised that md is using sector 64.
> > The superblock is at sectors 8,9
> > The bitmap at 16..24, or something like that
> > The data is at 262144 onwards.
> >
> > But that are offsets in sdb1, maybe the '64' is a sector number in 'sdb'.
> >
> > Where does 'sdb1' start in 'sdb'??
> >
> > NeilBrown
> >
> >
> >>
> >> Regards, Hans
> >>
> >> Am 29.10.2014 22:11, schrieb Hans Kraus:
> >>> Hi Neil,
> >>>
> >>> many thanks for your response. I rebooted the machine and got new drive
> >>> assignments. The drive in question is now sdc1. mdadm gave an error with
> >>> the dump, results below (nothing to report by dmesg) [by the way, I
> >>> wasn'nt even able to create an array on that drive with
> >>> 'mdadm --create --level=1 -n 2 /dev/sdc1 missing', same error(s)]:
> >>> -------------------------------------------------------------------------
> >>> root@nashorn:~# mdadm -E /dev/sdb1
> >>> /dev/sdb1:
> >>>             Magic : a92b4efc
> >>>           Version : 1.2
> >>>       Feature Map : 0x1
> >>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >>>              Name : nashorn:126  (local to host nashorn)
> >>>     Creation Time : Mon Oct 27 15:58:38 2014
> >>>        Raid Level : raid1
> >>>      Raid Devices : 2
> >>>
> >>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >>>        Array Size : 976630488 (931.39 GiB 1000.07 GB)
> >>>       Data Offset : 262144 sectors
> >>>      Super Offset : 8 sectors
> >>>      Unused Space : before=262056 sectors, after=0 sectors
> >>>             State : clean
> >>>       Device UUID : f7a91c46:e4cda0c0:1b770fd7:876634f8
> >>>
> >>> Internal Bitmap : 8 sectors from superblock
> >>>       Update Time : Wed Oct 29 18:42:16 2014
> >>>     Bad Block Log : 512 entries available at offset 72 sectors
> >>>          Checksum : 939b22d1 - correct
> >>>            Events : 3325
> >>>
> >>>
> >>>      Device Role : Active device 0
> >>>      Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
> >>> root@nashorn:~#
> >>> -------------------------------------------------------------------------
> >>> root@nashorn:~# mdadm -E /dev/sdh1
> >>> /dev/sdh1:
> >>>             Magic : a92b4efc
> >>>           Version : 1.2
> >>>       Feature Map : 0x1
> >>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >>>              Name : nashorn:126  (local to host nashorn)
> >>>     Creation Time : Mon Oct 27 15:58:38 2014
> >>>        Raid Level : raid1
> >>>      Raid Devices : 2
> >>>
> >>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >>>        Array Size : 976630488 (931.39 GiB 1000.07 GB)
> >>>       Data Offset : 262144 sectors
> >>>      Super Offset : 8 sectors
> >>>      Unused Space : before=262046 sectors, after=0 sectors
> >>>             State : clean
> >>>       Device UUID : 089d1d0e:4d5ca222:dd716acb:0813815c
> >>>
> >>> Internal Bitmap : 8 sectors from superblock
> >>>       Update Time : Wed Oct 29 18:42:16 2014
> >>>     Bad Block Log : 512 entries available at offset 82 sectors
> >>>          Checksum : 16454b96 - correct
> >>>            Events : 3325
> >>>
> >>>
> >>>      Device Role : Active device 1
> >>>      Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
> >>> root@nashorn:~#
> >>> -------------------------------------------------------------------------
> >>> root@nashorn:~# mdadm -E /dev/sdc1
> >>> /dev/sdc1:
> >>>             Magic : a92b4efc
> >>>           Version : 1.2
> >>>       Feature Map : 0x1
> >>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >>>              Name : nashorn:126  (local to host nashorn)
> >>>     Creation Time : Mon Oct 27 15:58:38 2014
> >>>        Raid Level : raid1
> >>>      Raid Devices : 2
> >>>
> >>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >>>        Array Size : 0
> >>>     Used Dev Size : 0
> >>>       Data Offset : 262144 sectors
> >>>      Super Offset : 8 sectors
> >>>      Unused Space : before=262056 sectors, after=1953260976 sectors
> >>>             State : clean
> >>>       Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
> >>>
> >>> Internal Bitmap : 8 sectors from superblock
> >>>       Update Time : Tue Oct 28 11:53:49 2014
> >>>     Bad Block Log : 512 entries available at offset 72 sectors
> >>>          Checksum : 6f02d040 - expected 891eedff
> >>>            Events : 0
> >>>
> >>>
> >>>      Device Role : spare
> >>>      Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
> >>> -------------------------------------------------------------------------
> >>> root@nashorn:~# mkdir -p /tmp/dump
> >>> root@nashorn:~# mdadm --dump /tmp/dump /dev/sdc1
> >>> mdadm: Failed to copy metadata from /dev/sdc1 to /tmp/dump/sdc1
> >>> -------------------------------------------------------------------------
> >>> Kind regards, Hans
> >>>
> >>> Am 28.10.2014 22:00, schrieb NeilBrown:
> >>>> On Tue, 28 Oct 2014 19:06:20 +0100 Hans Kraus <hans@hanswkraus.com>
> >>>> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> I debugged my problem a bit more: it seems that mdadm creates a defect
> >>>>> superblock. That's repeatedly happening, but only for one drive. I
> >>>>> copied zeros to that drive (via ddrescue /dev/zero ...) and the drive
> >>>>> looks OK. The info I got:
> >>>>>
> >>>>
> >>>> and you send me the metadata of all your devices please?
> >>>>
> >>>>      mkdir /tmp/dump
> >>>>      mdadm --dump /tmp/dump /dev/sde1
> >>>>      tar czvf /tmp/dump.tgz /tmp/dump
> >>>>
> >>>> and then send /tmp/dump.tgz
> >>>>
> >>>> Thanks.
> >>>>
> >>>> NeilBrown
> >>>>
> >>>> =================================================================================================================================================================================================
> >>>>
> >>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm --zero-superblock
> >>>>> /dev/sde1
> >>>>> mdadm: Unrecognised md component device - /dev/sde1
> >>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm --add /dev/md126
> >>>>> /dev/sde1
> >>>>> mdadm: add new device failed for /dev/sde1 as 4: Invalid argument
> >>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm -E /dev/sde1
> >>>>> /dev/sde1:
> >>>>>              Magic : a92b4efc
> >>>>>            Version : 1.2
> >>>>>        Feature Map : 0x1
> >>>>>         Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >>>>>               Name : nashorn:126  (local to host nashorn)
> >>>>>      Creation Time : Mon Oct 27 15:58:38 2014
> >>>>>         Raid Level : raid1
> >>>>>       Raid Devices : 2
> >>>>>
> >>>>>     Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >>>>>         Array Size : 0
> >>>>>      Used Dev Size : 0
> >>>>>        Data Offset : 262144 sectors
> >>>>>       Super Offset : 8 sectors
> >>>>>       Unused Space : before=262056 sectors, after=1953260976 sectors
> >>>>>              State : clean
> >>>>>        Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
> >>>>>
> >>>>> Internal Bitmap : 8 sectors from superblock
> >>>>>        Update Time : Tue Oct 28 11:53:49 2014
> >>>>>      Bad Block Log : 512 entries available at offset 72 sectors
> >>>>>           Checksum : 6f02d040 - expected 891eedff
> >>>>>             Events : 0
> >>>>>
> >>>>>
> >>>>>       Device Role : spare
> >>>>>       Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
> >>>>> root@nashorn:/home/kraush/work/smartctrl#
> >>>>> -----------------------------------------------------------------------------------------------------------
> >>>>>
> >>>>> root@nashorn:/home/kraush/work/smartctrl# cat /proc/mdstat
> >>>>> Personalities : [raid1] [raid6] [raid5] [raid4]
> >>>>> md127 : active (auto-read-only) raid6 sda1[0] sdk1[6](S) sdl2[7](S)
> >>>>> sdh1[5] sdg1[4] sdf1[3] sdc1[2] sdb1[1]
> >>>>>          1953017856 blocks super 1.2 level 6, 512k chunk, algorithm 2
> >>>>> [6/6] [UUUUUU]
> >>>>>          bitmap: 0/4 pages [0KB], 65536KB chunk
> >>>>>
> >>>>> md10 : active (auto-read-only) raid1 sdi2[0] sdl3[1]
> >>>>>          87833408 blocks super 1.2 [2/2] [UU]
> >>>>>          bitmap: 0/1 pages [0KB], 65536KB chunk
> >>>>>
> >>>>> md126 : active raid1 sdd1[3] sdj1[2]
> >>>>>          488254464 blocks super 1.2 [2/2] [UU]
> >>>>>          bitmap: 0/4 pages [0KB], 65536KB chunk
> >>>>>
> >>>>> md0 : active raid1 sdl1[4] sdd2[3] sdi1[5]
> >>>>>          156157824 blocks super 1.2 [3/1] [U__]
> >>>>>          [===================>.]  recovery = 95.6% (149290048/156157824)
> >>>>> finish=8.4min speed=13496K/sec
> >>>>>          bitmap: 1/2 pages [4KB], 65536KB chunk
> >>>>>
> >>>>> unused devices: <none>
> >>>>> root@nashorn:/home/kraush/work/smartctrl#
> >>>>> -----------------------------------------------------------------------------------------------------------
> >>>>>
> >>>>> dmesg:
> >>>>> [33299.387382] md: invalid superblock checksum on sde1
> >>>>> [33299.387385] md: sde1 does not have a valid v1.2 superblock, not
> >>>>> importing!
> >>>>> [33299.387408] md: md_import_device returned -22
> >>>>> root@nashorn:/home/kraush/work/smartctrl#
> >>>>> -----------------------------------------------------------------------------------------------------------
> >>>>>
> >>>>> root@nashorn:/home/kraush/work/mdadm# mdadm --version
> >>>>> mdadm - v3.3-161-gfed12d4 - 21st August 2014
> >>>>> =================================================================================================================================================================================================
> >>>>>
> >>>>>
> >>>>> Kind regards, Hans
> >>>>> --
> >>>>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >>>>> the body of a message to majordomo@vger.kernel.org
> >>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>>
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: Split RAID: Proposal for archival RAID using incremental batch checksum
From: NeilBrown @ 2014-11-24 22:50 UTC (permalink / raw)
  To: Anshuman Aggarwal; +Cc: Mdadm
In-Reply-To: <CAK-d5daq+-PbX1s81h01x7GdVDCijyzzPaeRAOCctEXTNideCw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7687 bytes --]

On Mon, 24 Nov 2014 12:59:47 +0530 Anshuman Aggarwal
<anshuman.aggarwal@gmail.com> wrote:

> On 3 November 2014 at 11:22, NeilBrown <neilb@suse.de> wrote:
> > On Thu, 30 Oct 2014 20:30:40 +0530 Anshuman Aggarwal
> > <anshuman.aggarwal@gmail.com> wrote:
> >
> >> Would chunksize==disksize work? Wouldn't that lead to the entire
> >> parity be invalidated for any write to any of the disks (assuming md
> >> operates at a chunk level)...also please see my reply below
> >
> > Operating at a chunk level would be a very poor design choice.  md/raid5
> > operates in units of 1 page (4K).
> 
> It appears that my requirement may be met by a partitionable md raid 4
> array where the partitions are all on individual underlying block
> devices not striped across the block devices. Is that currently
> possible with md raid? I dont' see how but such an enhancement could
> do all that I had outlined earlier
> 
> Is this possible to implement using RAID4 and MD already?

Nearly.  RAID4 currently requires the chunk size to be a power of 2.
Rounding down the size of your drives to match that could waste nearly half
the space.  However it should work as a proof-of-concept.

RAID0 supports non-power-of-2 chunk sizes.  Doing the same thing for
RAID4/5/6 would be quite possible.

>   can the
> partitions be made to write to individual block devices such that
> parity updates don't require reading all devices?

md/raid4 will currently tries to minimize total IO requests when performing
an update, but prefer spreading the IO over more devices if the total number
of requests is the same.

So for a 4-drive RAID4, Updating a single block can be done by:
  read old data block, read parity, write data, write parity - 4 IO requests
or
  read other 2 data blocks, write data, write parity - 4 IO requests.

In this case it will prefer the second, which is not what you want.
With 5-drive RAID4, the second option will require 5 IO requests, so the first
will be chosen.
It is quite trivial to flip this default for testing

-	if (rmw < rcw && rmw > 0) {
+	if (rmw <= rcw && rmw > 0) {


If you had 5 drives, you could experiment with no code changes.
Make the chunk size the largest power of 2 that fits in the device, and then
partition to align the partitions on those boundaries.

NeilBrown


> 
> To illustrate:
> -----------------RAID - 4 ---------------------
> |
> Device 1       Device 2       Device 3       Parity
> A1                 B1                 C1                P1
> A2                 B2                 C2                P2
> A3                 B3                 C3                P3
> 
> Each device gets written to independently (via a layer of block
> devices)...so Data on Device 1 is written as A1, A2, A3 contiguous
> blocks leading to updation of P1, P2 P3 (without causing any reads on
> devices 2 and 3 using XOR for the parity).
> 
> In RAID4, IIUC data gets striped and all devices become a single block device.
> 
> 
> >
> >
> >>
> >> On 29 October 2014 14:55, Anshuman Aggarwal <anshuman.aggarwal@gmail.com> wrote:
> >> > Right on most counts but please see comments below.
> >> >
> >> > On 29 October 2014 14:35, NeilBrown <neilb@suse.de> wrote:
> >> >> Just to be sure I understand, you would have N + X devices.  Each of the N
> >> >> devices contains an independent filesystem and could be accessed directly if
> >> >> needed.  Each of the X devices contains some codes so that if at most X
> >> >> devices in total died, you would still be able to recover all of the data.
> >> >> If more than X devices failed, you would still get complete data from the
> >> >> working devices.
> >> >>
> >> >> Every update would only write to the particular N device on which it is
> >> >> relevant, and  all of the X devices.  So N needs to be quite a bit bigger
> >> >> than X for the spin-down to be really worth it.
> >> >>
> >> >> Am I right so far?
> >> >
> >> > Perfectly right so far. I typically have a N to X ratio of 4 (4
> >> > devices to 1 data) so spin down is totally worth it for data
> >> > protection but more on that below.
> >> >
> >> >>
> >> >> For some reason the writes to X are delayed...  I don't really understand
> >> >> that part.
> >> >
> >> > This delay is basically designed around archival devices which are
> >> > rarely read from and even more rarely written to. By delaying writes
> >> > on 2 criteria ( designated cache buffer filling up or preset time
> >> > duration from last write expiring) we can significantly reduce the
> >> > writes on the parity device. This assumes that we are ok to lose a
> >> > movie or two in case the parity disk is not totally up to date but are
> >> > more interested in device longevity.
> >> >
> >> >>
> >> >> Sounds like multi-parity RAID6 with no parity rotation and
> >> >>   chunksize == devicesize
> >> > RAID6 would present us with a joint device and currently only allows
> >> > writes to that directly, yes? Any writes will be striped.
> >
> > If the chunksize equals the device size, then you need a very large write for
> > it to be striped.
> >
> >> > In any case would md raid allow the underlying device to be written to
> >> > directly? Also how would it know that the device has been written to
> >> > and hence parity has to be updated? What about the superblock which
> >> > the FS would not know about?
> >
> > No, you wouldn't write to the underlying device.  You would carefully
> > partition the RAID5 so each partition aligns exactly with an underlying
> > device.  Then write to the partition.
> >
> >> >
> >> > Also except for the delayed checksum writing part which would be
> >> > significant if one of the objectives is to reduce the amount of
> >> > writes. Can we delay that in the code currently for RAID6? I
> >> > understand the objective of RAID6 is to ensure data recovery and we
> >> > are looking at a compromise in this case.
> >
> > "simple matter of programming"
> > Of course there would be a limit to how much data can be buffered in memory
> > before it has to be flushed out.
> > If you are mostly storing movies, then they are probably too large to
> > buffer.  Why not just write them out straight away?
> >
> > NeilBrown
> >
> >
> >
> >> >
> >> > If feasible, this can be an enhancement to MD RAID as well where N
> >> > devices are presented instead of a single joint device in case of
> >> > raid6 (maybe the multi part device can be individual disks?)
> >> >
> >> > It will certainly solve my problem of where to store the metadata. I
> >> > was currently hoping to just store it as a configuration file to be
> >> > read by the initramfs since in this case worst case scenario the
> >> > checksum goes out of sync and is rebuilt from scratch.
> >> >
> >> >>
> >> >> I wouldn't use device-mapper myself, but you are unlikely to get an entirely
> >> >> impartial opinion from me on that topic.
> >> >
> >> > I haven't hacked around the kernel internals much so far so will have
> >> > to dig out that history. I will welcome any particular links/mail
> >> > threads I should look at for guidance (with both yours and opposing
> >> > points of view)
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* md raid performance with 3-18-rc3
From: Manish Awasthi @ 2014-11-24  8:10 UTC (permalink / raw)
  To: linux-raid

Hi,

We benchmarked the md raid driver performance on 3-18-rc3 kernel and 
compared the results with that of 3.6.11. The reason for this exercise 
is to understand if multithreaded raid driver has any performance 
benefits over 3.6.11 which is single threaded. Here are some details 
about the setup

System: Intel(R) Xeon(R) CPU E3-1230 V2 @ 3.30GHz 4 cores (8threads), 
8GB RAM.
Setup: 3 SSDs create a raid5 array
test tool: iozone (only read/re-read, write/re-write tested), blocksize: 
4k-64k, filesize: 1Gig to 200Gig

Comparison was done for speed of data transfer in kBytes/sec and also 
the CPU utilization as reported by iozone.

raid on 3.18.0-rc3 performed much worse than raid on 3.6.11.

Read/Write: raid on 3.18.0-rc3 operated at almost half the speed of raid 
on 3.6.11

CPU Utilization: With md raid on 3.18.0-rc3, the CPU utilization was 
less than half of md raid on 3.6.11 on WRITE operations. However, for 
READ operations, 3.18-0.rc3 had more CPU utilization than 3.6.11.

Also, I noticed that scaling up the CPU cores of the system scales down 
the raid througput with 3.18.0-rc3.

I do have detailed logs of the comparison but I'm not sure I should send 
those on this mailing list.

If my observation aligns with someone else's, then what is really the 
gain with multithreaded raid.

Manish

^ permalink raw reply

* Re: Split RAID: Proposal for archival RAID using incremental batch checksum
From: Anshuman Aggarwal @ 2014-11-24  7:29 UTC (permalink / raw)
  To: NeilBrown; +Cc: Mdadm
In-Reply-To: <20141103165217.3bfd3d3e@notabene.brown>

On 3 November 2014 at 11:22, NeilBrown <neilb@suse.de> wrote:
> On Thu, 30 Oct 2014 20:30:40 +0530 Anshuman Aggarwal
> <anshuman.aggarwal@gmail.com> wrote:
>
>> Would chunksize==disksize work? Wouldn't that lead to the entire
>> parity be invalidated for any write to any of the disks (assuming md
>> operates at a chunk level)...also please see my reply below
>
> Operating at a chunk level would be a very poor design choice.  md/raid5
> operates in units of 1 page (4K).

It appears that my requirement may be met by a partitionable md raid 4
array where the partitions are all on individual underlying block
devices not striped across the block devices. Is that currently
possible with md raid? I dont' see how but such an enhancement could
do all that I had outlined earlier

Is this possible to implement using RAID4 and MD already? can the
partitions be made to write to individual block devices such that
parity updates don't require reading all devices?

To illustrate:
-----------------RAID - 4 ---------------------
|
Device 1       Device 2       Device 3       Parity
A1                 B1                 C1                P1
A2                 B2                 C2                P2
A3                 B3                 C3                P3

Each device gets written to independently (via a layer of block
devices)...so Data on Device 1 is written as A1, A2, A3 contiguous
blocks leading to updation of P1, P2 P3 (without causing any reads on
devices 2 and 3 using XOR for the parity).

In RAID4, IIUC data gets striped and all devices become a single block device.


>
>
>>
>> On 29 October 2014 14:55, Anshuman Aggarwal <anshuman.aggarwal@gmail.com> wrote:
>> > Right on most counts but please see comments below.
>> >
>> > On 29 October 2014 14:35, NeilBrown <neilb@suse.de> wrote:
>> >> Just to be sure I understand, you would have N + X devices.  Each of the N
>> >> devices contains an independent filesystem and could be accessed directly if
>> >> needed.  Each of the X devices contains some codes so that if at most X
>> >> devices in total died, you would still be able to recover all of the data.
>> >> If more than X devices failed, you would still get complete data from the
>> >> working devices.
>> >>
>> >> Every update would only write to the particular N device on which it is
>> >> relevant, and  all of the X devices.  So N needs to be quite a bit bigger
>> >> than X for the spin-down to be really worth it.
>> >>
>> >> Am I right so far?
>> >
>> > Perfectly right so far. I typically have a N to X ratio of 4 (4
>> > devices to 1 data) so spin down is totally worth it for data
>> > protection but more on that below.
>> >
>> >>
>> >> For some reason the writes to X are delayed...  I don't really understand
>> >> that part.
>> >
>> > This delay is basically designed around archival devices which are
>> > rarely read from and even more rarely written to. By delaying writes
>> > on 2 criteria ( designated cache buffer filling up or preset time
>> > duration from last write expiring) we can significantly reduce the
>> > writes on the parity device. This assumes that we are ok to lose a
>> > movie or two in case the parity disk is not totally up to date but are
>> > more interested in device longevity.
>> >
>> >>
>> >> Sounds like multi-parity RAID6 with no parity rotation and
>> >>   chunksize == devicesize
>> > RAID6 would present us with a joint device and currently only allows
>> > writes to that directly, yes? Any writes will be striped.
>
> If the chunksize equals the device size, then you need a very large write for
> it to be striped.
>
>> > In any case would md raid allow the underlying device to be written to
>> > directly? Also how would it know that the device has been written to
>> > and hence parity has to be updated? What about the superblock which
>> > the FS would not know about?
>
> No, you wouldn't write to the underlying device.  You would carefully
> partition the RAID5 so each partition aligns exactly with an underlying
> device.  Then write to the partition.
>
>> >
>> > Also except for the delayed checksum writing part which would be
>> > significant if one of the objectives is to reduce the amount of
>> > writes. Can we delay that in the code currently for RAID6? I
>> > understand the objective of RAID6 is to ensure data recovery and we
>> > are looking at a compromise in this case.
>
> "simple matter of programming"
> Of course there would be a limit to how much data can be buffered in memory
> before it has to be flushed out.
> If you are mostly storing movies, then they are probably too large to
> buffer.  Why not just write them out straight away?
>
> NeilBrown
>
>
>
>> >
>> > If feasible, this can be an enhancement to MD RAID as well where N
>> > devices are presented instead of a single joint device in case of
>> > raid6 (maybe the multi part device can be individual disks?)
>> >
>> > It will certainly solve my problem of where to store the metadata. I
>> > was currently hoping to just store it as a configuration file to be
>> > read by the initramfs since in this case worst case scenario the
>> > checksum goes out of sync and is rebuilt from scratch.
>> >
>> >>
>> >> I wouldn't use device-mapper myself, but you are unlikely to get an entirely
>> >> impartial opinion from me on that topic.
>> >
>> > I haven't hacked around the kernel internals much so far so will have
>> > to dig out that history. I will welcome any particular links/mail
>> > threads I should look at for guidance (with both yours and opposing
>> > points of view)
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* MD RAID5 hang on make_request
From: Henry Cai @ 2014-11-23  1:08 UTC (permalink / raw)
  To: linux-raid

With kernel-3.10, when raid5 is doing resync, unplug one of the
raid5's disk, RAID5 will hang on raid5's make_request function, and
cat /proc/mdstat show the RAID "resync=PENDING", mdadm version is
3.3.2. The platform is arm.

Any similar problems?

^ permalink raw reply

* MD component device renaming with udev and MD on full disk
From: Jason Keltz @ 2014-11-21 21:42 UTC (permalink / raw)
  To: linux-raid

Hi.

I have two questions about MD..

1) I've written a udev rule to remap /dev/sdX devices on my system to 
/dev/cXeYsZpA (controller, enclosure, slot, partition) mapping. When I 
reboot the system, I see that all the devices in /dev are appropriately 
renamed.  If I look at /proc/mdstat, it still has the kernel names of 
the devices (/dev/sdX) even though those devices no longer exist.  If I 
do an mdadm --detail /dev/mdX the system reports the proper device name 
makeup.  I manually failed a device, and I got the correct device name 
in the email.  I'm just wondering what command I would execute to make 
/proc/mdstat update the devices in its output?

2) Unrelated to 1) -- the argument re: using MD on full devices versus 
partitions has been around for a long time.  I've been experimenting 
with using it on full devices.  One of the arguments that I've read for 
not using full devices is that apparently, if you have two devices that 
are identical, but one of them is slightly smaller than the other due to 
say, bad sectors, then these disks can't be used together in one MD 
because they are different sizes. I'm wondering how valid that argument 
is?  Surely it would make sense if MD was using full devices for it to 
actually stop short of the end of the disk for situations like these...

Thanks for any help you can provide.

Jason.

^ permalink raw reply

* Re: degraded raid troubleshooting
From: Phil Turmel @ 2014-11-20 23:16 UTC (permalink / raw)
  To: Stephen Burke, linux-raid
In-Reply-To: <CAAugdn4bp9im=hk=8qWkcxpzss-dzB-5Fq-3KnJH6y3AoqXxZQ@mail.gmail.com>

Hi Stephen,

On 11/20/2014 08:41 AM, Stephen Burke wrote:
> I woke up this morning to my pc not booting saying that my raid was in
> a degraded state.  I looked at the raid wiki and it told me to stop
> what I was doing and mail the linux-raid list before doing anything
> hasty.

:-)

> Here's all the info that I could find out about it.  Any help would be
> appreciated.
> I am running Ubuntu 12.04
> mdadm - v3.2.5 - 18th May 2012
> 
> The drive in question is /dev/sdb1 on my system.  I tried to look at
> it via fdisk but it hangs up.  What should my first steps to figure
> out if this drive is bad and if so replace it.  Thanks.

Good news: your data is still safe, and already assembled (ready to
use).  The boot failure is a one-time warning that the number of drives
available at shutdown didn't match the available drives at bootup.

> syslog
> 
> Nov 20 01:14:53 ht-pc kernel: [    2.465076]          res
> 41/40:08:09:08:00/00:00:00:00:00/00 Emask 0x409 (media error) <F>
> 
> Nov 20 01:14:53 ht-pc kernel: [    2.465078] ata2.00: status: { DRDY ERR }
> 
> Nov 20 01:14:53 ht-pc kernel: [    2.465079] ata2.00: error: { UNC }
> 
> Nov 20 01:14:53 ht-pc kernel: [    2.484536] ata2.00: configured for UDMA/133
> 
> Nov 20 01:14:53 ht-pc kernel: [    2.484543] ata2: EH complete
> 
> Nov 20 01:14:53 ht-pc kernel: [    3.131754] ata2.00: exception Emask
> 0x0 SAct 0x40 SErr 0x0 action 0x0
> 
> Nov 20 01:14:53 ht-pc kernel: [    3.131756] ata2.00: irq_stat 0x40000008
> 
> Nov 20 01:14:53 ht-pc kernel: [    3.131758] ata2.00: failed command:
> READ FPDMA QUEUED
> 
> Nov 20 01:14:53 ht-pc kernel: [    3.131762] ata2.00: cmd
> 60/08:30:08:08:00/00:00:00:00:00/40 tag 6 ncq 4096 in
> 
> Nov 20 01:14:53 ht-pc kernel: [    3.131763]          res
> 41/40:08:09:08:00/00:00:00:00:00/00 Emask 0x409 (media error) <F>

Bad news: that drive is very likely dead.  It didn't communicate at all.

If you replace the drive and the replacement works, I would count that
as definitively a bad drive.  But it could be a cable or controller
problem.  Such things happen.

Before adding the new drive, though, I would show the "mdadm -E" reports
for each of the surviving member devices.  Just in case you encounter a
problem during rebuild (ridiculously common for big drives in raid5).

Anyways, use "mdadm /dev/md0 --add /dev/sdX1" after you partition the
new drive.  That'll start the rebuild.

Phil



^ permalink raw reply

* [PATCH v2 5/5] imsm: use efivarfs interface for reading UEFI variables
From: Artur Paszkiewicz @ 2014-11-20 17:56 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, pawel.baldysiak, Artur Paszkiewicz

Read UEFI variables using the new efivarfs interface, fallback to
sysfs-efivars if that fails.

Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
 platform-intel.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/platform-intel.c b/platform-intel.c
index 54ef37f..37274da 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -416,6 +416,7 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
 
 #define SYS_EFI_VAR_PATH "/sys/firmware/efi/vars"
+#define SYS_EFIVARS_PATH "/sys/firmware/efi/efivars"
 #define SCU_PROP "RstScuV"
 #define AHCI_PROP "RstSataV"
 #define AHCI_SSATA_PROP "RstsSatV"
@@ -426,13 +427,47 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
 
 #define PCI_CLASS_RAID_CNTRL 0x010400
 
-int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
+static int read_efi_var(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
+{
+	char path[PATH_MAX];
+	char buf[GUID_STR_MAX];
+	int fd;
+	ssize_t n;
+
+	snprintf(path, PATH_MAX, "%s/%s-%s", SYS_EFIVARS_PATH, variable_name, guid_str(buf, guid));
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return 1;
+
+	/* read the variable attributes and ignore it */
+	n = read(fd, buf, sizeof(__u32));
+	if (n < 0) {
+		close(fd);
+		return 1;
+	}
+
+	/* read the variable data */
+	n = read(fd, buffer, buf_size);
+	close(fd);
+	if (n < buf_size)
+		return 1;
+
+	return 0;
+}
+
+static int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
 {
 	char path[PATH_MAX];
 	char buf[GUID_STR_MAX];
 	int dfd;
 	ssize_t n, var_data_len;
 
+	/* Try to read the variable using the new efivarfs interface first.
+	 * If that fails, fall back to the old sysfs-efivars interface. */
+	if (!read_efi_var(buffer, buf_size, variable_name, guid))
+		return 0;
+
 	snprintf(path, PATH_MAX, "%s/%s-%s/size", SYS_EFI_VAR_PATH, variable_name, guid_str(buf, guid));
 
 	dprintf("EFI VAR: path=%s\n", path);
-- 
1.8.4.5


^ permalink raw reply related

* Re: [PATCH 1/5] imsm: support for OROMs shared by multiple HBAs
From: Artur Paszkiewicz @ 2014-11-20 17:50 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid, pawel.baldysiak
In-Reply-To: <20141120140748.6daa763a@notabene.brown>

On 11/20/2014 04:07 AM, NeilBrown wrote:
> On Wed, 19 Nov 2014 13:53:26 +0100 Artur Paszkiewicz
> <artur.paszkiewicz@intel.com> wrote:
> 
>> HBAs can share OROMs (e.g. SATA/sSATA). They are matched by PCI device
>> id. Removed populated_orom/efi and imsm_orom/efi arrays - they are
>> replaced by oroms array and functions get_orom_by_device_id(),
>> add_orom(), add_orom_device_id().
>>
>> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> 
> Hi,
>  this patch seems to make a lot more changes that the above brief description
>  seems to suggest.
>  Is there any chance of breaking it up into two or three parts, or at least
>  describing everything that is being changed.
> 
>  I'm half tempted to just accept it as it is, as it is just "your" code that
>  that is being changed, but I'd like to understand it if I can.
> 
> Thanks,
> NeilBrown
> 

Hi Neil,

Splitting this up reasonably turned out to be more difficult than I
thought, so I'll try to provide a more detailed description of the
changes. 

The IMSM platform code was based on an assumption that the OROM or UEFI
capability structure (represented by struct imsm_orom) always belongs to
only one HBA. This assumption is no longer valid, because of newer
platforms with dual AHCI HBAs. Each HBA can have a separate OROM, but
some versions have a combined OROM for both HBAs.

This patch implements this HBA-OROM relationship in struct orom_entry,
which matches an OROM with a list of HBA PCI ids. All the detected
orom_entries are stored and retrieved using a global array and the
functions add_orom(), add_orom_device_id() and get_orom_by_device_id().
This replaces the arrays: imsm_orom, populated_orom, imsm_efi,
populated_efi.

The scan() function is extended to find all HBAs for an OROM. The list
of their device ids is retrieved from the PCI Expansion ROM Data
Structure, hence the additional field devListOffset in struct
pciExpDataStructFormat.

In UEFI mode we can't read the PCI Expansion ROM Data Structure and the
imsm_orom structures are stored in UEFI variables. They do not provide a
similar device id list, so we also check the HBA PCI class to make sure
that the HBA has RAID mode enabled.

In super-intel.c there are changes which allow spanning of IMSM
containers over HBAs of the same type, but only if the HBAs share the
same OROM.  This is done by comparing imsm_orom pointers, which (outside
of platform-intel.c) always point to the global array containing all the
detected oroms. Additional warnings are added to
validate_container_imsm() to warn about potentially dangerous operations
in all the possible cases, e.g. when an array is assembled using disks
attached to HBAs with separate OROMs.

I hope you find this description helpful and that it will make the
changes easier to understand.

Regards,
Artur

> 
>> ---
>>  platform-intel.c | 248 ++++++++++++++++++++++++++++++++-----------------------
>>  platform-intel.h |   5 +-
>>  super-intel.c    | 134 +++++++++++++++++++++---------
>>  3 files changed, 243 insertions(+), 144 deletions(-)
>>
>> diff --git a/platform-intel.c b/platform-intel.c
>> index f347382..f779d02 100644
>> --- a/platform-intel.c
>> +++ b/platform-intel.c
>> @@ -59,6 +59,7 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>>  	struct sys_dev *list = NULL;
>>  	enum sys_dev_type type;
>>  	unsigned long long dev_id;
>> +	unsigned long long class;
>>  
>>  	if (strcmp(driver, "isci") == 0)
>>  		type = SYS_DEV_SAS;
>> @@ -99,6 +100,9 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>>  		if (devpath_to_ll(path, "device", &dev_id) != 0)
>>  			continue;
>>  
>> +		if (devpath_to_ll(path, "class", &class) != 0)
>> +			continue;
>> +
>>  		/* start / add list entry */
>>  		if (!head) {
>>  			head = xmalloc(sizeof(*head));
>> @@ -114,6 +118,7 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>>  		}
>>  
>>  		list->dev_id = (__u16) dev_id;
>> +		list->class = (__u32) class;
>>  		list->type = type;
>>  		list->path = realpath(path, NULL);
>>  		list->next = NULL;
>> @@ -127,16 +132,6 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>>  static struct sys_dev *intel_devices=NULL;
>>  static time_t valid_time = 0;
>>  
>> -static enum sys_dev_type device_type_by_id(__u16 device_id)
>> -{
>> -	struct sys_dev *iter;
>> -
>> -	for(iter = intel_devices; iter != NULL; iter = iter->next)
>> -		if (iter->dev_id == device_id)
>> -			return iter->type;
>> -	return SYS_DEV_UNKNOWN;
>> -}
>> -
>>  static int devpath_to_ll(const char *dev_path, const char *entry, unsigned long long *val)
>>  {
>>  	char path[strlen(dev_path) + strlen(entry) + 2];
>> @@ -209,16 +204,79 @@ struct pciExpDataStructFormat {
>>  	__u8  ver[4];
>>  	__u16 vendorID;
>>  	__u16 deviceID;
>> +	__u16 devListOffset;
>>  } __attribute__ ((packed));
>>  
>> -static struct imsm_orom imsm_orom[SYS_DEV_MAX];
>> -static int populated_orom[SYS_DEV_MAX];
>> +struct devid_list {
>> +	__u16 devid;
>> +	struct devid_list *next;
>> +};
>> +
>> +struct orom_entry {
>> +	struct imsm_orom orom;
>> +	struct devid_list *devid_list;
>> +};
>> +
>> +static struct orom_entry oroms[SYS_DEV_MAX];
>> +
>> +const struct imsm_orom *get_orom_by_device_id(__u16 dev_id)
>> +{
>> +	int i;
>> +	struct devid_list *list;
>> +
>> +	for (i = 0; i < SYS_DEV_MAX; i++) {
>> +		for (list = oroms[i].devid_list; list; list = list->next) {
>> +			if (list->devid == dev_id)
>> +				return &oroms[i].orom;
>> +		}
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < SYS_DEV_MAX; i++) {
>> +		if (&oroms[i].orom == orom)
>> +			return orom;
>> +		if (oroms[i].orom.signature[0] == 0) {
>> +			oroms[i].orom = *orom;
>> +			return &oroms[i].orom;
>> +		}
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static void add_orom_device_id(const struct imsm_orom *orom, __u16 dev_id)
>> +{
>> +	int i;
>> +	struct devid_list *list;
>> +	struct devid_list *prev = NULL;
>> +
>> +	for (i = 0; i < SYS_DEV_MAX; i++) {
>> +		if (&oroms[i].orom == orom) {
>> +			for (list = oroms[i].devid_list; list; prev = list, list = list->next) {
>> +				if (list->devid == dev_id)
>> +					return;
>> +			}
>> +			list = xmalloc(sizeof(struct devid_list));
>> +			list->devid = dev_id;
>> +			list->next = NULL;
>> +
>> +			if (prev == NULL)
>> +				oroms[i].devid_list = list;
>> +			else
>> +				prev->next = list;
>> +			return;
>> +		}
>> +	}
>> +}
>>  
>>  static int scan(const void *start, const void *end, const void *data)
>>  {
>>  	int offset;
>> -	const struct imsm_orom *imsm_mem;
>> -	int dev;
>> +	const struct imsm_orom *imsm_mem = NULL;
>>  	int len = (end - start);
>>  	struct pciExpDataStructFormat *ptr= (struct pciExpDataStructFormat *)data;
>>  
>> @@ -231,81 +289,83 @@ static int scan(const void *start, const void *end, const void *data)
>>  		(ulong) __le16_to_cpu(ptr->vendorID),
>>  		(ulong) __le16_to_cpu(ptr->deviceID));
>>  
>> -	if (__le16_to_cpu(ptr->vendorID) == 0x8086) {
>> -		/* serach  attached intel devices by device id from OROM */
>> -		dev = device_type_by_id(__le16_to_cpu(ptr->deviceID));
>> -		if (dev == SYS_DEV_UNKNOWN)
>> -			return 0;
>> -	}
>> -	else
>> +	if (__le16_to_cpu(ptr->vendorID) != 0x8086)
>>  		return 0;
>>  
>>  	for (offset = 0; offset < len; offset += 4) {
>> -		imsm_mem = start + offset;
>> -		if ((memcmp(imsm_mem->signature, "$VER", 4) == 0)) {
>> -			imsm_orom[dev] = *imsm_mem;
>> -			populated_orom[dev] = 1;
>> -			return populated_orom[SYS_DEV_SATA] && populated_orom[SYS_DEV_SAS];
>> +		const void *mem = start + offset;
>> +
>> +		if ((memcmp(mem, IMSM_OROM_SIGNATURE, 4) == 0)) {
>> +			imsm_mem = mem;
>> +			break;
>>  		}
>>  	}
>> +
>> +	if (!imsm_mem)
>> +		return 0;
>> +
>> +	const struct imsm_orom *orom = add_orom(imsm_mem);
>> +
>> +	if (ptr->devListOffset) {
>> +		const __u16 *dev_list = (void *)ptr + ptr->devListOffset;
>> +		int i;
>> +
>> +		for (i = 0; dev_list[i] != 0; i++)
>> +			add_orom_device_id(orom, dev_list[i]);
>> +	} else {
>> +		add_orom_device_id(orom, __le16_to_cpu(ptr->deviceID));
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> -const struct imsm_orom *imsm_platform_test(enum sys_dev_type hba_id, int *populated,
>> -					   struct imsm_orom *imsm_orom)
>> +const struct imsm_orom *imsm_platform_test(struct sys_dev *hba)
>>  {
>> -	memset(imsm_orom, 0, sizeof(*imsm_orom));
>> -	imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>> -				IMSM_OROM_RLC_RAID10 | IMSM_OROM_RLC_RAID5;
>> -	imsm_orom->sss = IMSM_OROM_SSS_4kB | IMSM_OROM_SSS_8kB |
>> -				IMSM_OROM_SSS_16kB | IMSM_OROM_SSS_32kB |
>> -				IMSM_OROM_SSS_64kB | IMSM_OROM_SSS_128kB |
>> -				IMSM_OROM_SSS_256kB | IMSM_OROM_SSS_512kB |
>> -				IMSM_OROM_SSS_1MB | IMSM_OROM_SSS_2MB;
>> -	imsm_orom->dpa = IMSM_OROM_DISKS_PER_ARRAY;
>> -	imsm_orom->tds = IMSM_OROM_TOTAL_DISKS;
>> -	imsm_orom->vpa = IMSM_OROM_VOLUMES_PER_ARRAY;
>> -	imsm_orom->vphba = IMSM_OROM_VOLUMES_PER_HBA;
>> -	imsm_orom->attr = imsm_orom->rlc | IMSM_OROM_ATTR_ChecksumVerify;
>> -	*populated = 1;
>> +	struct imsm_orom orom = {
>> +		.signature = IMSM_OROM_SIGNATURE,
>> +		.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>> +					IMSM_OROM_RLC_RAID10 | IMSM_OROM_RLC_RAID5,
>> +		.sss = IMSM_OROM_SSS_4kB | IMSM_OROM_SSS_8kB |
>> +					IMSM_OROM_SSS_16kB | IMSM_OROM_SSS_32kB |
>> +					IMSM_OROM_SSS_64kB | IMSM_OROM_SSS_128kB |
>> +					IMSM_OROM_SSS_256kB | IMSM_OROM_SSS_512kB |
>> +					IMSM_OROM_SSS_1MB | IMSM_OROM_SSS_2MB,
>> +		.dpa = IMSM_OROM_DISKS_PER_ARRAY,
>> +		.tds = IMSM_OROM_TOTAL_DISKS,
>> +		.vpa = IMSM_OROM_VOLUMES_PER_ARRAY,
>> +		.vphba = IMSM_OROM_VOLUMES_PER_HBA
>> +	};
>> +	orom.attr = orom.rlc | IMSM_OROM_ATTR_ChecksumVerify;
>>  
>>  	if (check_env("IMSM_TEST_OROM_NORAID5")) {
>> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>>  				IMSM_OROM_RLC_RAID10;
>>  	}
>> -	if (check_env("IMSM_TEST_AHCI_EFI_NORAID5") && (hba_id == SYS_DEV_SAS)) {
>> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>> +	if (check_env("IMSM_TEST_AHCI_EFI_NORAID5") && (hba->type == SYS_DEV_SAS)) {
>> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>>  				IMSM_OROM_RLC_RAID10;
>>  	}
>> -	if (check_env("IMSM_TEST_SCU_EFI_NORAID5") && (hba_id == SYS_DEV_SATA)) {
>> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>> +	if (check_env("IMSM_TEST_SCU_EFI_NORAID5") && (hba->type == SYS_DEV_SATA)) {
>> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>>  				IMSM_OROM_RLC_RAID10;
>>  	}
>>  
>> -	return imsm_orom;
>> +	const struct imsm_orom *ret = add_orom(&orom);
>> +
>> +	add_orom_device_id(ret, hba->dev_id);
>> +
>> +	return ret;
>>  }
>>  
>> -static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
>> +static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
>>  {
>>  	unsigned long align;
>>  
>> -	if (hba_id >= SYS_DEV_MAX)
>> -		return NULL;
>> +	if (check_env("IMSM_TEST_OROM"))
>> +		return imsm_platform_test(hba);
>>  
>> -	/* it's static data so we only need to read it once */
>> -	if (populated_orom[hba_id]) {
>> -		dprintf("OROM CAP: %p, pid: %d pop: %d\n",
>> -			&imsm_orom[hba_id], (int) getpid(), populated_orom[hba_id]);
>> -		return &imsm_orom[hba_id];
>> -	}
>> -	if (check_env("IMSM_TEST_OROM")) {
>> -		dprintf("OROM CAP: %p,  pid: %d pop: %d\n",
>> -			&imsm_orom[hba_id], (int) getpid(), populated_orom[hba_id]);
>> -		return imsm_platform_test(hba_id, &populated_orom[hba_id], &imsm_orom[hba_id]);
>> -	}
>>  	/* return empty OROM capabilities in EFI test mode */
>> -	if (check_env("IMSM_TEST_AHCI_EFI") ||
>> -	    check_env("IMSM_TEST_SCU_EFI"))
>> +	if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
>>  		return NULL;
>>  
>>  	find_intel_devices();
>> @@ -325,9 +385,7 @@ static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
>>  	scan_adapter_roms(scan);
>>  	probe_roms_exit();
>>  
>> -	if (populated_orom[hba_id])
>> -		return &imsm_orom[hba_id];
>> -	return NULL;
>> +	return get_orom_by_device_id(hba->dev_id);
>>  }
>>  
>>  #define GUID_STR_MAX	37  /* according to GUID format:
>> @@ -347,9 +405,7 @@ static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
>>  #define VENDOR_GUID \
>>  	EFI_GUID(0x193dfefa, 0xa445, 0x4302, 0x99, 0xd8, 0xef, 0x3a, 0xad, 0x1a, 0x04, 0xc6)
>>  
>> -int populated_efi[SYS_DEV_MAX] = { 0, 0 };
>> -
>> -static struct imsm_orom imsm_efi[SYS_DEV_MAX];
>> +#define PCI_CLASS_RAID_CNTRL 0x010400
>>  
>>  int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
>>  {
>> @@ -395,54 +451,40 @@ int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struc
>>  	return 0;
>>  }
>>  
>> -const struct imsm_orom *find_imsm_efi(enum sys_dev_type hba_id)
>> +const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
>>  {
>> -	if (hba_id >= SYS_DEV_MAX)
>> -		return NULL;
>> +	struct imsm_orom orom;
>> +	const struct imsm_orom *ret;
>>  
>> -	dprintf("EFI CAP: %p,  pid: %d pop: %d\n",
>> -		&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
>> +	if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
>> +		return imsm_platform_test(hba);
>>  
>> -	/* it's static data so we only need to read it once */
>> -	if (populated_efi[hba_id]) {
>> -		dprintf("EFI CAP: %p, pid: %d pop: %d\n",
>> -			&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
>> -		return &imsm_efi[hba_id];
>> -	}
>> -	if (check_env("IMSM_TEST_AHCI_EFI") ||
>> -	    check_env("IMSM_TEST_SCU_EFI")) {
>> -		dprintf("OROM CAP: %p,  pid: %d pop: %d\n",
>> -			&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
>> -		return imsm_platform_test(hba_id, &populated_efi[hba_id], &imsm_efi[hba_id]);
>> -	}
>>  	/* OROM test is set, return that there is no EFI capabilities */
>>  	if (check_env("IMSM_TEST_OROM"))
>>  		return NULL;
>>  
>> -	if (read_efi_variable(&imsm_efi[hba_id], sizeof(imsm_efi[0]), hba_id == SYS_DEV_SAS ? SCU_PROP : AHCI_PROP, VENDOR_GUID)) {
>> -		populated_efi[hba_id] = 0;
>> +	if (hba->type == SYS_DEV_SATA && hba->class != PCI_CLASS_RAID_CNTRL)
>>  		return NULL;
>> -	}
>>  
>> -	populated_efi[hba_id] = 1;
>> -	return &imsm_efi[hba_id];
>> -}
>> +	if (read_efi_variable(&orom, sizeof(orom), hba->type == SYS_DEV_SAS ? SCU_PROP : AHCI_PROP, VENDOR_GUID))
>> +		return NULL;
>>  
>> -/*
>> - * backward interface compatibility
>> - */
>> -const struct imsm_orom *find_imsm_orom(void)
>> -{
>> -	return find_imsm_hba_orom(SYS_DEV_SATA);
>> +	ret = add_orom(&orom);
>> +	add_orom_device_id(ret, hba->dev_id);
>> +
>> +	return ret;
>>  }
>>  
>> -const struct imsm_orom *find_imsm_capability(enum sys_dev_type hba_id)
>> +const struct imsm_orom *find_imsm_capability(struct sys_dev *hba)
>>  {
>> -	const struct imsm_orom *cap=NULL;
>> +	const struct imsm_orom *cap = get_orom_by_device_id(hba->dev_id);
>> +
>> +	if (cap)
>> +		return cap;
>>  
>> -	if ((cap = find_imsm_efi(hba_id)) != NULL)
>> +	if ((cap = find_imsm_efi(hba)) != NULL)
>>  		return cap;
>> -	if ((cap = find_imsm_hba_orom(hba_id)) != NULL)
>> +	if ((cap = find_imsm_hba_orom(hba)) != NULL)
>>  		return cap;
>>  	return NULL;
>>  }
>> diff --git a/platform-intel.h b/platform-intel.h
>> index 8226be3..e41f386 100644
>> --- a/platform-intel.h
>> +++ b/platform-intel.h
>> @@ -22,6 +22,7 @@
>>  /* The IMSM Capability (IMSM AHCI and ISCU OROM/EFI variable) Version Table definition */
>>  struct imsm_orom {
>>  	__u8 signature[4];
>> +	#define IMSM_OROM_SIGNATURE "$VER"
>>  	__u8 table_ver_major; /* Currently 2 (can change with future revs) */
>>  	__u8 table_ver_minor; /* Currently 2 (can change with future revs) */
>>  	__u16 major_ver; /* Example: 8 as in 8.6.0.1020 */
>> @@ -180,6 +181,7 @@ struct sys_dev {
>>  	char *path;
>>  	char *pci_id;
>>  	__u16  dev_id;
>> +	__u32  class;
>>  	struct sys_dev *next;
>>  };
>>  
>> @@ -201,10 +203,11 @@ static inline char *guid_str(char *buf, struct efi_guid guid)
>>  char *diskfd_to_devpath(int fd);
>>  struct sys_dev *find_driver_devices(const char *bus, const char *driver);
>>  struct sys_dev *find_intel_devices(void);
>> -const struct imsm_orom *find_imsm_capability(enum sys_dev_type hba_id);
>> +const struct imsm_orom *find_imsm_capability(struct sys_dev *hba);
>>  const struct imsm_orom *find_imsm_orom(void);
>>  int disk_attached_to_hba(int fd, const char *hba_path);
>>  int devt_attached_to_hba(dev_t dev, const char *hba_path);
>>  char *devt_to_devpath(dev_t dev);
>>  int path_attached_to_hba(const char *disk_path, const char *hba_path);
>>  const char *get_sys_dev_type(enum sys_dev_type);
>> +const struct imsm_orom *get_orom_by_device_id(__u16 device_id);
>> diff --git a/super-intel.c b/super-intel.c
>> index e28ac7d..dabf011 100644
>> --- a/super-intel.c
>> +++ b/super-intel.c
>> @@ -555,11 +555,26 @@ static int attach_hba_to_super(struct intel_super *super, struct sys_dev *device
>>  	if (super->hba == NULL) {
>>  		super->hba = alloc_intel_hba(device);
>>  		return 1;
>> -	} else
>> -		/* IMSM metadata disallows to attach disks to multiple
>> -		 * controllers.
>> -		 */
>> +	}
>> +
>> +	hba = super->hba;
>> +	/* Intel metadata allows for all disks attached to the same type HBA.
>> +	 * Do not sypport odf HBA types mixing
>> +	 */
>> +	if (device->type != hba->type)
>> +		return 2;
>> +
>> +	/* Multiple same type HBAs can be used if they share the same OROM */
>> +	const struct imsm_orom *device_orom = get_orom_by_device_id(device->dev_id);
>> +
>> +	if (device_orom != super->orom)
>>  		return 2;
>> +
>> +	while (hba->next)
>> +		hba = hba->next;
>> +
>> +	hba->next = alloc_intel_hba(device);
>> +	return 1;
>>  }
>>  
>>  static struct sys_dev* find_disk_attached_hba(int fd, const char *devname)
>> @@ -1886,13 +1901,12 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
>>  		if (!list)
>>  			return 2;
>>  		for (hba = list; hba; hba = hba->next) {
>> -			orom = find_imsm_capability(hba->type);
>> -			if (!orom) {
>> -				result = 2;
>> +			if (find_imsm_capability(hba)) {
>> +				result = 0;
>>  				break;
>>  			}
>>  			else
>> -				result = 0;
>> +				result = 2;
>>  		}
>>  		return result;
>>  	}
>> @@ -1909,7 +1923,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
>>  	for (hba = list; hba; hba = hba->next) {
>>  		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
>>  			continue;
>> -		orom = find_imsm_capability(hba->type);
>> +		orom = find_imsm_capability(hba);
>>  		if (!orom)
>>  			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
>>  				hba->path, get_sys_dev_type(hba->type));
>> @@ -1954,7 +1968,7 @@ static int export_detail_platform_imsm(int verbose, char *controller_path)
>>  	for (hba = list; hba; hba = hba->next) {
>>  		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
>>  			continue;
>> -		orom = find_imsm_capability(hba->type);
>> +		orom = find_imsm_capability(hba);
>>  		if (!orom) {
>>  			if (verbose > 0)
>>  				pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICE_UNDER_%s\n",hba->path);
>> @@ -3087,13 +3101,18 @@ static int compare_super_imsm(struct supertype *st, struct supertype *tst)
>>  	 * use the same Intel hba
>>  	 * If not on Intel hba at all, allow anything.
>>  	 */
>> -	if (!check_env("IMSM_NO_PLATFORM")) {
>> -		if (first->hba && sec->hba &&
>> -		    strcmp(first->hba->path, sec->hba->path) != 0)  {
>> +	if (!check_env("IMSM_NO_PLATFORM") && first->hba && sec->hba) {
>> +		if (first->hba->type != sec->hba->type) {
>> +			fprintf(stderr,
>> +				"HBAs of devices do not match %s != %s\n",
>> +				get_sys_dev_type(first->hba->type),
>> +				get_sys_dev_type(sec->hba->type));
>> +			return 3;
>> +		}
>> +		if (first->orom != sec->orom) {
>>  			fprintf(stderr,
>> -				"HBAs of devices does not match %s != %s\n",
>> -				first->hba ? first->hba->path : NULL,
>> -				sec->hba ? sec->hba->path : NULL);
>> +				"HBAs of devices do not match %s != %s\n",
>> +				first->hba->pci_id, sec->hba->pci_id);
>>  			return 3;
>>  		}
>>  	}
>> @@ -3832,14 +3851,13 @@ static int find_intel_hba_capability(int fd, struct intel_super *super, char *de
>>  					fprintf(stderr, ", ");
>>  				hba = hba->next;
>>  			}
>> -
>> -			fprintf(stderr, ").\n");
>> -			cont_err("Mixing devices attached to multiple controllers "
>> -				 "is not allowed.\n");
>> +			fprintf(stderr, ").\n"
>> +				"    Mixing devices attached to different controllers "
>> +				"is not allowed.\n");
>>  		}
>>  		return 2;
>>  	}
>> -	super->orom = find_imsm_capability(hba_name->type);
>> +	super->orom = find_imsm_capability(hba_name);
>>  	if (!super->orom)
>>  		return 3;
>>  	return 0;
>> @@ -9061,32 +9079,68 @@ int open_backup_targets(struct mdinfo *info, int raid_disks, int *raid_fds,
>>   ******************************************************************************/
>>  int validate_container_imsm(struct mdinfo *info)
>>  {
>> -	if (!check_env("IMSM_NO_PLATFORM")) {
>> -		struct sys_dev *idev;
>> -		struct mdinfo *dev;
>> -		char *hba_path = NULL;
>> -		char *dev_path = devt_to_devpath(makedev(info->disk.major,
>> -										info->disk.minor));
>> +	if (check_env("IMSM_NO_PLATFORM"))
>> +		return 0;
>>  
>> -		for (idev = find_intel_devices(); idev; idev = idev->next) {
>> -			if (strstr(dev_path, idev->path)) {
>> -				hba_path = idev->path;
>> -				break;
>> -			}
>> +	struct sys_dev *idev;
>> +	struct sys_dev *hba = NULL;
>> +	struct sys_dev *intel_devices = find_intel_devices();
>> +	char *dev_path = devt_to_devpath(makedev(info->disk.major,
>> +									info->disk.minor));
>> +
>> +	for (idev = intel_devices; idev; idev = idev->next) {
>> +		if (dev_path && strstr(dev_path, idev->path)) {
>> +			hba = idev;
>> +			break;
>>  		}
>> +	}
>> +	if (dev_path)
>>  		free(dev_path);
>>  
>> -		if (hba_path) {
>> -			for (dev = info->next; dev; dev = dev->next) {
>> -				if (!devt_attached_to_hba(makedev(dev->disk.major,
>> -						dev->disk.minor), hba_path)) {
>> -					pr_err("WARNING - IMSM container assembled with disks under different HBAs!\n"
>> -						"       This operation is not supported and can lead to data loss.\n");
>> -					return 1;
>> -				}
>> +	if (!hba) {
>> +		pr_err("WARNING - Cannot detect HBA for device %s!\n",
>> +				devid2kname(makedev(info->disk.major, info->disk.minor)));
>> +		return 1;
>> +	}
>> +
>> +	const struct imsm_orom *orom = get_orom_by_device_id(hba->dev_id);
>> +	struct mdinfo *dev;
>> +
>> +	for (dev = info->next; dev; dev = dev->next) {
>> +		dev_path = devt_to_devpath(makedev(dev->disk.major, dev->disk.minor));
>> +
>> +		struct sys_dev *hba2 = NULL;
>> +		for (idev = intel_devices; idev; idev = idev->next) {
>> +			if (dev_path && strstr(dev_path, idev->path)) {
>> +				hba2 = idev;
>> +				break;
>>  			}
>>  		}
>> +		if (dev_path)
>> +			free(dev_path);
>> +
>> +		const struct imsm_orom *orom2 = hba2 == NULL ? NULL :
>> +				get_orom_by_device_id(hba2->dev_id);
>> +
>> +		if (hba2 && hba->type != hba2->type) {
>> +			pr_err("WARNING - HBAs of devices do not match %s != %s\n",
>> +				get_sys_dev_type(hba->type), get_sys_dev_type(hba2->type));
>> +			return 1;
>> +		}
>> +
>> +		if (orom != orom2) {
>> +			pr_err("WARNING - IMSM container assembled with disks under different HBAs!\n"
>> +				"       This operation is not supported and can lead to data loss.\n");
>> +			return 1;
>> +		}
>> +
>> +		if (!orom) {
>> +			pr_err("WARNING - IMSM container assembled with disks under HBAs without IMSM platform support!\n"
>> +				"       This operation is not supported and can lead to data loss.\n");
>> +			return 1;
>> +		}
>>  	}
>> +
>>  	return 0;
>>  }
>>  #ifndef MDASSEMBLE
> 


^ permalink raw reply

* Re: mdadm creates corrupt superblock
From: Hans Kraus @ 2014-11-20 15:58 UTC (permalink / raw)
  To: NeilBrown; +Cc: Linux-RAID
In-Reply-To: <20141120084709.36583d80@notabene.brown>

Hi Neil,

the partition table (output of 'sfdisk -d /dev/sdh'):

# partition table of /dev/sdh
unit: sectors

/dev/sdh1 : start=     2048, size=1953523120, Id=83
/dev/sdh2 : start=        0, size=        0, Id= 0
/dev/sdh3 : start=        0, size=        0, Id= 0
/dev/sdh4 : start=        0, size=        0, Id= 0

Makes that any sense to you?

Regards, Hans

On 11/19/2014 10:47 PM, NeilBrown wrote:
> On Wed, 19 Nov 2014 10:55:05 +0100 Hans Kraus <hans@hanswkraus.com> wrote:
>
>> Hi,
>>
>> I think I found the problem. 'badblocks' reports sector 64 of the HD as
>> errornous. But the HD neither reports a read error nor remaps that
>> sector.
>>
>> Is it possible to parametrize mdadm that it doesn't use that sector?
>
> I'm surprised that md is using sector 64.
> The superblock is at sectors 8,9
> The bitmap at 16..24, or something like that
> The data is at 262144 onwards.
>
> But that are offsets in sdb1, maybe the '64' is a sector number in 'sdb'.
>
> Where does 'sdb1' start in 'sdb'??
>
> NeilBrown
>
>
>>
>> Regards, Hans
>>
>> Am 29.10.2014 22:11, schrieb Hans Kraus:
>>> Hi Neil,
>>>
>>> many thanks for your response. I rebooted the machine and got new drive
>>> assignments. The drive in question is now sdc1. mdadm gave an error with
>>> the dump, results below (nothing to report by dmesg) [by the way, I
>>> wasn'nt even able to create an array on that drive with
>>> 'mdadm --create --level=1 -n 2 /dev/sdc1 missing', same error(s)]:
>>> -------------------------------------------------------------------------
>>> root@nashorn:~# mdadm -E /dev/sdb1
>>> /dev/sdb1:
>>>             Magic : a92b4efc
>>>           Version : 1.2
>>>       Feature Map : 0x1
>>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
>>>              Name : nashorn:126  (local to host nashorn)
>>>     Creation Time : Mon Oct 27 15:58:38 2014
>>>        Raid Level : raid1
>>>      Raid Devices : 2
>>>
>>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
>>>        Array Size : 976630488 (931.39 GiB 1000.07 GB)
>>>       Data Offset : 262144 sectors
>>>      Super Offset : 8 sectors
>>>      Unused Space : before=262056 sectors, after=0 sectors
>>>             State : clean
>>>       Device UUID : f7a91c46:e4cda0c0:1b770fd7:876634f8
>>>
>>> Internal Bitmap : 8 sectors from superblock
>>>       Update Time : Wed Oct 29 18:42:16 2014
>>>     Bad Block Log : 512 entries available at offset 72 sectors
>>>          Checksum : 939b22d1 - correct
>>>            Events : 3325
>>>
>>>
>>>      Device Role : Active device 0
>>>      Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
>>> root@nashorn:~#
>>> -------------------------------------------------------------------------
>>> root@nashorn:~# mdadm -E /dev/sdh1
>>> /dev/sdh1:
>>>             Magic : a92b4efc
>>>           Version : 1.2
>>>       Feature Map : 0x1
>>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
>>>              Name : nashorn:126  (local to host nashorn)
>>>     Creation Time : Mon Oct 27 15:58:38 2014
>>>        Raid Level : raid1
>>>      Raid Devices : 2
>>>
>>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
>>>        Array Size : 976630488 (931.39 GiB 1000.07 GB)
>>>       Data Offset : 262144 sectors
>>>      Super Offset : 8 sectors
>>>      Unused Space : before=262046 sectors, after=0 sectors
>>>             State : clean
>>>       Device UUID : 089d1d0e:4d5ca222:dd716acb:0813815c
>>>
>>> Internal Bitmap : 8 sectors from superblock
>>>       Update Time : Wed Oct 29 18:42:16 2014
>>>     Bad Block Log : 512 entries available at offset 82 sectors
>>>          Checksum : 16454b96 - correct
>>>            Events : 3325
>>>
>>>
>>>      Device Role : Active device 1
>>>      Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
>>> root@nashorn:~#
>>> -------------------------------------------------------------------------
>>> root@nashorn:~# mdadm -E /dev/sdc1
>>> /dev/sdc1:
>>>             Magic : a92b4efc
>>>           Version : 1.2
>>>       Feature Map : 0x1
>>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
>>>              Name : nashorn:126  (local to host nashorn)
>>>     Creation Time : Mon Oct 27 15:58:38 2014
>>>        Raid Level : raid1
>>>      Raid Devices : 2
>>>
>>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
>>>        Array Size : 0
>>>     Used Dev Size : 0
>>>       Data Offset : 262144 sectors
>>>      Super Offset : 8 sectors
>>>      Unused Space : before=262056 sectors, after=1953260976 sectors
>>>             State : clean
>>>       Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
>>>
>>> Internal Bitmap : 8 sectors from superblock
>>>       Update Time : Tue Oct 28 11:53:49 2014
>>>     Bad Block Log : 512 entries available at offset 72 sectors
>>>          Checksum : 6f02d040 - expected 891eedff
>>>            Events : 0
>>>
>>>
>>>      Device Role : spare
>>>      Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
>>> -------------------------------------------------------------------------
>>> root@nashorn:~# mkdir -p /tmp/dump
>>> root@nashorn:~# mdadm --dump /tmp/dump /dev/sdc1
>>> mdadm: Failed to copy metadata from /dev/sdc1 to /tmp/dump/sdc1
>>> -------------------------------------------------------------------------
>>> Kind regards, Hans
>>>
>>> Am 28.10.2014 22:00, schrieb NeilBrown:
>>>> On Tue, 28 Oct 2014 19:06:20 +0100 Hans Kraus <hans@hanswkraus.com>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I debugged my problem a bit more: it seems that mdadm creates a defect
>>>>> superblock. That's repeatedly happening, but only for one drive. I
>>>>> copied zeros to that drive (via ddrescue /dev/zero ...) and the drive
>>>>> looks OK. The info I got:
>>>>>
>>>>
>>>> and you send me the metadata of all your devices please?
>>>>
>>>>      mkdir /tmp/dump
>>>>      mdadm --dump /tmp/dump /dev/sde1
>>>>      tar czvf /tmp/dump.tgz /tmp/dump
>>>>
>>>> and then send /tmp/dump.tgz
>>>>
>>>> Thanks.
>>>>
>>>> NeilBrown
>>>>
>>>> =================================================================================================================================================================================================
>>>>
>>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm --zero-superblock
>>>>> /dev/sde1
>>>>> mdadm: Unrecognised md component device - /dev/sde1
>>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm --add /dev/md126
>>>>> /dev/sde1
>>>>> mdadm: add new device failed for /dev/sde1 as 4: Invalid argument
>>>>> root@nashorn:/home/kraush/work/smartctrl# mdadm -E /dev/sde1
>>>>> /dev/sde1:
>>>>>              Magic : a92b4efc
>>>>>            Version : 1.2
>>>>>        Feature Map : 0x1
>>>>>         Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
>>>>>               Name : nashorn:126  (local to host nashorn)
>>>>>      Creation Time : Mon Oct 27 15:58:38 2014
>>>>>         Raid Level : raid1
>>>>>       Raid Devices : 2
>>>>>
>>>>>     Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
>>>>>         Array Size : 0
>>>>>      Used Dev Size : 0
>>>>>        Data Offset : 262144 sectors
>>>>>       Super Offset : 8 sectors
>>>>>       Unused Space : before=262056 sectors, after=1953260976 sectors
>>>>>              State : clean
>>>>>        Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
>>>>>
>>>>> Internal Bitmap : 8 sectors from superblock
>>>>>        Update Time : Tue Oct 28 11:53:49 2014
>>>>>      Bad Block Log : 512 entries available at offset 72 sectors
>>>>>           Checksum : 6f02d040 - expected 891eedff
>>>>>             Events : 0
>>>>>
>>>>>
>>>>>       Device Role : spare
>>>>>       Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
>>>>> root@nashorn:/home/kraush/work/smartctrl#
>>>>> -----------------------------------------------------------------------------------------------------------
>>>>>
>>>>> root@nashorn:/home/kraush/work/smartctrl# cat /proc/mdstat
>>>>> Personalities : [raid1] [raid6] [raid5] [raid4]
>>>>> md127 : active (auto-read-only) raid6 sda1[0] sdk1[6](S) sdl2[7](S)
>>>>> sdh1[5] sdg1[4] sdf1[3] sdc1[2] sdb1[1]
>>>>>          1953017856 blocks super 1.2 level 6, 512k chunk, algorithm 2
>>>>> [6/6] [UUUUUU]
>>>>>          bitmap: 0/4 pages [0KB], 65536KB chunk
>>>>>
>>>>> md10 : active (auto-read-only) raid1 sdi2[0] sdl3[1]
>>>>>          87833408 blocks super 1.2 [2/2] [UU]
>>>>>          bitmap: 0/1 pages [0KB], 65536KB chunk
>>>>>
>>>>> md126 : active raid1 sdd1[3] sdj1[2]
>>>>>          488254464 blocks super 1.2 [2/2] [UU]
>>>>>          bitmap: 0/4 pages [0KB], 65536KB chunk
>>>>>
>>>>> md0 : active raid1 sdl1[4] sdd2[3] sdi1[5]
>>>>>          156157824 blocks super 1.2 [3/1] [U__]
>>>>>          [===================>.]  recovery = 95.6% (149290048/156157824)
>>>>> finish=8.4min speed=13496K/sec
>>>>>          bitmap: 1/2 pages [4KB], 65536KB chunk
>>>>>
>>>>> unused devices: <none>
>>>>> root@nashorn:/home/kraush/work/smartctrl#
>>>>> -----------------------------------------------------------------------------------------------------------
>>>>>
>>>>> dmesg:
>>>>> [33299.387382] md: invalid superblock checksum on sde1
>>>>> [33299.387385] md: sde1 does not have a valid v1.2 superblock, not
>>>>> importing!
>>>>> [33299.387408] md: md_import_device returned -22
>>>>> root@nashorn:/home/kraush/work/smartctrl#
>>>>> -----------------------------------------------------------------------------------------------------------
>>>>>
>>>>> root@nashorn:/home/kraush/work/mdadm# mdadm --version
>>>>> mdadm - v3.3-161-gfed12d4 - 21st August 2014
>>>>> =================================================================================================================================================================================================
>>>>>
>>>>>
>>>>> Kind regards, Hans
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply

* degraded raid troubleshooting
From: Stephen Burke @ 2014-11-20 13:41 UTC (permalink / raw)
  To: linux-raid

I woke up this morning to my pc not booting saying that my raid was in
a degraded state.  I looked at the raid wiki and it told me to stop
what I was doing and mail the linux-raid list before doing anything
hasty.

Here's all the info that I could find out about it.  Any help would be
appreciated.
I am running Ubuntu 12.04
mdadm - v3.2.5 - 18th May 2012

The drive in question is /dev/sdb1 on my system.  I tried to look at
it via fdisk but it hangs up.  What should my first steps to figure
out if this drive is bad and if so replace it.  Thanks.


sburke@ht-pc:/tmp/logs$ sudo mdadm --detail /dev/md0

[sudo] password for sburke:

/dev/md0:

        Version : 1.2

  Creation Time : Fri Dec 13 01:18:13 2013

     Raid Level : raid5

     Array Size : 3906763776 (3725.78 GiB 4000.53 GB)

  Used Dev Size : 1953381888 (1862.89 GiB 2000.26 GB)

   Raid Devices : 3

  Total Devices : 2

    Persistence : Superblock is persistent


    Update Time : Thu Nov 20 01:16:00 2014

          State : clean, degraded

 Active Devices : 2

Working Devices : 2

 Failed Devices : 0

  Spare Devices : 0


         Layout : left-symmetric

     Chunk Size : 512K


           Name : ht-pc:0  (local to host ht-pc)

           UUID : 508cb42f:d2c1ea9c:e62b4121:c3d9cbc3

         Events : 140


    Number   Major   Minor   RaidDevice State

       0       0        0        0      removed

       1       8       33        1      active sync   /dev/sdc1

       3       8       65        2      active sync   /dev/sde1

sburke@ht-pc:/tmp/logs$ cat /proc/mdstat

Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5]
[raid4] [raid10]

md0 : active raid5 sde1[3] sdc1[1]

      3906763776 blocks super 1.2 level 5, 512k chunk, algorithm 2 [3/2] [_UU]



unused devices: <none>

syslog

Nov 20 01:14:53 ht-pc kernel: [    2.465076]          res
41/40:08:09:08:00/00:00:00:00:00/00 Emask 0x409 (media error) <F>

Nov 20 01:14:53 ht-pc kernel: [    2.465078] ata2.00: status: { DRDY ERR }

Nov 20 01:14:53 ht-pc kernel: [    2.465079] ata2.00: error: { UNC }

Nov 20 01:14:53 ht-pc kernel: [    2.484536] ata2.00: configured for UDMA/133

Nov 20 01:14:53 ht-pc kernel: [    2.484543] ata2: EH complete

Nov 20 01:14:53 ht-pc kernel: [    3.131754] ata2.00: exception Emask
0x0 SAct 0x40 SErr 0x0 action 0x0

Nov 20 01:14:53 ht-pc kernel: [    3.131756] ata2.00: irq_stat 0x40000008

Nov 20 01:14:53 ht-pc kernel: [    3.131758] ata2.00: failed command:
READ FPDMA QUEUED

Nov 20 01:14:53 ht-pc kernel: [    3.131762] ata2.00: cmd
60/08:30:08:08:00/00:00:00:00:00/40 tag 6 ncq 4096 in

Nov 20 01:14:53 ht-pc kernel: [    3.131763]          res
41/40:08:09:08:00/00:00:00:00:00/00 Emask 0x409 (media error) <F>

-- 
Steve
www.stayathomedevs.com

Game Data Editor Unity Plugin

^ permalink raw reply

* Re: [PATCH 5/5] imsm: use efivarfs interface for reading UEFI variables
From: NeilBrown @ 2014-11-20  3:11 UTC (permalink / raw)
  To: Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak
In-Reply-To: <1416401610-16209-6-git-send-email-artur.paszkiewicz@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

On Wed, 19 Nov 2014 13:53:30 +0100 Artur Paszkiewicz
<artur.paszkiewicz@intel.com> wrote:

> Read UEFI variables using the new efivarfs interface, fallback to
> sysfs-efivars if that fails.
> 
> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> ---
>  platform-intel.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/platform-intel.c b/platform-intel.c
> index 54ef37f..586a2f6 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -416,6 +416,7 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
>    (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
>  
>  #define SYS_EFI_VAR_PATH "/sys/firmware/efi/vars"
> +#define SYS_EFIVARS_PATH "/sys/firmware/efi/efivars"
>  #define SCU_PROP "RstScuV"
>  #define AHCI_PROP "RstSataV"
>  #define AHCI_SSATA_PROP "RstsSatV"
> @@ -426,10 +427,44 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
>  
>  #define PCI_CLASS_RAID_CNTRL 0x010400
>  
> -int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
> +static int read_efi_var(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
>  {
>  	char path[PATH_MAX];
>  	char buf[GUID_STR_MAX];
> +	int fd;
> +	ssize_t n;
> +
> +	snprintf(path, PATH_MAX, "%s/%s-%s", SYS_EFIVARS_PATH, variable_name, guid_str(buf, guid));
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return 1;
> +
> +	/* read the variable attributes and ignore it */
> +	n = read(fd, buf, sizeof(__u32));
> +	if (n < 0) {
> +		close(fd);
> +		return 1;
> +	}
> +
> +	/* read the variable data */
> +	n = read(fd, buffer, buf_size);
> +	close(fd);
> +	if (n < buf_size)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
> +{
> +	/* Try to read the variable using the new efivarfs interface first.
> +	 * If that fails, fall back to the old sysfs-efivars interface. */
> +	if (!read_efi_var(buffer, buf_size, variable_name, guid))
> +		return 0;
> +
> +	char path[PATH_MAX];
> +	char buf[GUID_STR_MAX];
>  	int dfd;
>  	ssize_t n, var_data_len;
>  


Patch 2, 3, 4 look OK.
This one is nearly OK, but I don't like to see executable code (the
read_efi_var call) before variable declarations (even though some versions of
C allow it).

So if you can put that 'if' *after* the variables, it will be OK.

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH 1/5] imsm: support for OROMs shared by multiple HBAs
From: NeilBrown @ 2014-11-20  3:07 UTC (permalink / raw)
  To: Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak
In-Reply-To: <1416401610-16209-2-git-send-email-artur.paszkiewicz@intel.com>

[-- Attachment #1: Type: text/plain, Size: 21863 bytes --]

On Wed, 19 Nov 2014 13:53:26 +0100 Artur Paszkiewicz
<artur.paszkiewicz@intel.com> wrote:

> HBAs can share OROMs (e.g. SATA/sSATA). They are matched by PCI device
> id. Removed populated_orom/efi and imsm_orom/efi arrays - they are
> replaced by oroms array and functions get_orom_by_device_id(),
> add_orom(), add_orom_device_id().
> 
> Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>

Hi,
 this patch seems to make a lot more changes that the above brief description
 seems to suggest.
 Is there any chance of breaking it up into two or three parts, or at least
 describing everything that is being changed.

 I'm half tempted to just accept it as it is, as it is just "your" code that
 that is being changed, but I'd like to understand it if I can.

Thanks,
NeilBrown


> ---
>  platform-intel.c | 248 ++++++++++++++++++++++++++++++++-----------------------
>  platform-intel.h |   5 +-
>  super-intel.c    | 134 +++++++++++++++++++++---------
>  3 files changed, 243 insertions(+), 144 deletions(-)
> 
> diff --git a/platform-intel.c b/platform-intel.c
> index f347382..f779d02 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -59,6 +59,7 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>  	struct sys_dev *list = NULL;
>  	enum sys_dev_type type;
>  	unsigned long long dev_id;
> +	unsigned long long class;
>  
>  	if (strcmp(driver, "isci") == 0)
>  		type = SYS_DEV_SAS;
> @@ -99,6 +100,9 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>  		if (devpath_to_ll(path, "device", &dev_id) != 0)
>  			continue;
>  
> +		if (devpath_to_ll(path, "class", &class) != 0)
> +			continue;
> +
>  		/* start / add list entry */
>  		if (!head) {
>  			head = xmalloc(sizeof(*head));
> @@ -114,6 +118,7 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>  		}
>  
>  		list->dev_id = (__u16) dev_id;
> +		list->class = (__u32) class;
>  		list->type = type;
>  		list->path = realpath(path, NULL);
>  		list->next = NULL;
> @@ -127,16 +132,6 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
>  static struct sys_dev *intel_devices=NULL;
>  static time_t valid_time = 0;
>  
> -static enum sys_dev_type device_type_by_id(__u16 device_id)
> -{
> -	struct sys_dev *iter;
> -
> -	for(iter = intel_devices; iter != NULL; iter = iter->next)
> -		if (iter->dev_id == device_id)
> -			return iter->type;
> -	return SYS_DEV_UNKNOWN;
> -}
> -
>  static int devpath_to_ll(const char *dev_path, const char *entry, unsigned long long *val)
>  {
>  	char path[strlen(dev_path) + strlen(entry) + 2];
> @@ -209,16 +204,79 @@ struct pciExpDataStructFormat {
>  	__u8  ver[4];
>  	__u16 vendorID;
>  	__u16 deviceID;
> +	__u16 devListOffset;
>  } __attribute__ ((packed));
>  
> -static struct imsm_orom imsm_orom[SYS_DEV_MAX];
> -static int populated_orom[SYS_DEV_MAX];
> +struct devid_list {
> +	__u16 devid;
> +	struct devid_list *next;
> +};
> +
> +struct orom_entry {
> +	struct imsm_orom orom;
> +	struct devid_list *devid_list;
> +};
> +
> +static struct orom_entry oroms[SYS_DEV_MAX];
> +
> +const struct imsm_orom *get_orom_by_device_id(__u16 dev_id)
> +{
> +	int i;
> +	struct devid_list *list;
> +
> +	for (i = 0; i < SYS_DEV_MAX; i++) {
> +		for (list = oroms[i].devid_list; list; list = list->next) {
> +			if (list->devid == dev_id)
> +				return &oroms[i].orom;
> +		}
> +	}
> +	return NULL;
> +}
> +
> +static const struct imsm_orom *add_orom(const struct imsm_orom *orom)
> +{
> +	int i;
> +
> +	for (i = 0; i < SYS_DEV_MAX; i++) {
> +		if (&oroms[i].orom == orom)
> +			return orom;
> +		if (oroms[i].orom.signature[0] == 0) {
> +			oroms[i].orom = *orom;
> +			return &oroms[i].orom;
> +		}
> +	}
> +	return NULL;
> +}
> +
> +static void add_orom_device_id(const struct imsm_orom *orom, __u16 dev_id)
> +{
> +	int i;
> +	struct devid_list *list;
> +	struct devid_list *prev = NULL;
> +
> +	for (i = 0; i < SYS_DEV_MAX; i++) {
> +		if (&oroms[i].orom == orom) {
> +			for (list = oroms[i].devid_list; list; prev = list, list = list->next) {
> +				if (list->devid == dev_id)
> +					return;
> +			}
> +			list = xmalloc(sizeof(struct devid_list));
> +			list->devid = dev_id;
> +			list->next = NULL;
> +
> +			if (prev == NULL)
> +				oroms[i].devid_list = list;
> +			else
> +				prev->next = list;
> +			return;
> +		}
> +	}
> +}
>  
>  static int scan(const void *start, const void *end, const void *data)
>  {
>  	int offset;
> -	const struct imsm_orom *imsm_mem;
> -	int dev;
> +	const struct imsm_orom *imsm_mem = NULL;
>  	int len = (end - start);
>  	struct pciExpDataStructFormat *ptr= (struct pciExpDataStructFormat *)data;
>  
> @@ -231,81 +289,83 @@ static int scan(const void *start, const void *end, const void *data)
>  		(ulong) __le16_to_cpu(ptr->vendorID),
>  		(ulong) __le16_to_cpu(ptr->deviceID));
>  
> -	if (__le16_to_cpu(ptr->vendorID) == 0x8086) {
> -		/* serach  attached intel devices by device id from OROM */
> -		dev = device_type_by_id(__le16_to_cpu(ptr->deviceID));
> -		if (dev == SYS_DEV_UNKNOWN)
> -			return 0;
> -	}
> -	else
> +	if (__le16_to_cpu(ptr->vendorID) != 0x8086)
>  		return 0;
>  
>  	for (offset = 0; offset < len; offset += 4) {
> -		imsm_mem = start + offset;
> -		if ((memcmp(imsm_mem->signature, "$VER", 4) == 0)) {
> -			imsm_orom[dev] = *imsm_mem;
> -			populated_orom[dev] = 1;
> -			return populated_orom[SYS_DEV_SATA] && populated_orom[SYS_DEV_SAS];
> +		const void *mem = start + offset;
> +
> +		if ((memcmp(mem, IMSM_OROM_SIGNATURE, 4) == 0)) {
> +			imsm_mem = mem;
> +			break;
>  		}
>  	}
> +
> +	if (!imsm_mem)
> +		return 0;
> +
> +	const struct imsm_orom *orom = add_orom(imsm_mem);
> +
> +	if (ptr->devListOffset) {
> +		const __u16 *dev_list = (void *)ptr + ptr->devListOffset;
> +		int i;
> +
> +		for (i = 0; dev_list[i] != 0; i++)
> +			add_orom_device_id(orom, dev_list[i]);
> +	} else {
> +		add_orom_device_id(orom, __le16_to_cpu(ptr->deviceID));
> +	}
> +
>  	return 0;
>  }
>  
> -const struct imsm_orom *imsm_platform_test(enum sys_dev_type hba_id, int *populated,
> -					   struct imsm_orom *imsm_orom)
> +const struct imsm_orom *imsm_platform_test(struct sys_dev *hba)
>  {
> -	memset(imsm_orom, 0, sizeof(*imsm_orom));
> -	imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
> -				IMSM_OROM_RLC_RAID10 | IMSM_OROM_RLC_RAID5;
> -	imsm_orom->sss = IMSM_OROM_SSS_4kB | IMSM_OROM_SSS_8kB |
> -				IMSM_OROM_SSS_16kB | IMSM_OROM_SSS_32kB |
> -				IMSM_OROM_SSS_64kB | IMSM_OROM_SSS_128kB |
> -				IMSM_OROM_SSS_256kB | IMSM_OROM_SSS_512kB |
> -				IMSM_OROM_SSS_1MB | IMSM_OROM_SSS_2MB;
> -	imsm_orom->dpa = IMSM_OROM_DISKS_PER_ARRAY;
> -	imsm_orom->tds = IMSM_OROM_TOTAL_DISKS;
> -	imsm_orom->vpa = IMSM_OROM_VOLUMES_PER_ARRAY;
> -	imsm_orom->vphba = IMSM_OROM_VOLUMES_PER_HBA;
> -	imsm_orom->attr = imsm_orom->rlc | IMSM_OROM_ATTR_ChecksumVerify;
> -	*populated = 1;
> +	struct imsm_orom orom = {
> +		.signature = IMSM_OROM_SIGNATURE,
> +		.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
> +					IMSM_OROM_RLC_RAID10 | IMSM_OROM_RLC_RAID5,
> +		.sss = IMSM_OROM_SSS_4kB | IMSM_OROM_SSS_8kB |
> +					IMSM_OROM_SSS_16kB | IMSM_OROM_SSS_32kB |
> +					IMSM_OROM_SSS_64kB | IMSM_OROM_SSS_128kB |
> +					IMSM_OROM_SSS_256kB | IMSM_OROM_SSS_512kB |
> +					IMSM_OROM_SSS_1MB | IMSM_OROM_SSS_2MB,
> +		.dpa = IMSM_OROM_DISKS_PER_ARRAY,
> +		.tds = IMSM_OROM_TOTAL_DISKS,
> +		.vpa = IMSM_OROM_VOLUMES_PER_ARRAY,
> +		.vphba = IMSM_OROM_VOLUMES_PER_HBA
> +	};
> +	orom.attr = orom.rlc | IMSM_OROM_ATTR_ChecksumVerify;
>  
>  	if (check_env("IMSM_TEST_OROM_NORAID5")) {
> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>  				IMSM_OROM_RLC_RAID10;
>  	}
> -	if (check_env("IMSM_TEST_AHCI_EFI_NORAID5") && (hba_id == SYS_DEV_SAS)) {
> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
> +	if (check_env("IMSM_TEST_AHCI_EFI_NORAID5") && (hba->type == SYS_DEV_SAS)) {
> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>  				IMSM_OROM_RLC_RAID10;
>  	}
> -	if (check_env("IMSM_TEST_SCU_EFI_NORAID5") && (hba_id == SYS_DEV_SATA)) {
> -		imsm_orom->rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
> +	if (check_env("IMSM_TEST_SCU_EFI_NORAID5") && (hba->type == SYS_DEV_SATA)) {
> +		orom.rlc = IMSM_OROM_RLC_RAID0 | IMSM_OROM_RLC_RAID1 |
>  				IMSM_OROM_RLC_RAID10;
>  	}
>  
> -	return imsm_orom;
> +	const struct imsm_orom *ret = add_orom(&orom);
> +
> +	add_orom_device_id(ret, hba->dev_id);
> +
> +	return ret;
>  }
>  
> -static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
> +static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
>  {
>  	unsigned long align;
>  
> -	if (hba_id >= SYS_DEV_MAX)
> -		return NULL;
> +	if (check_env("IMSM_TEST_OROM"))
> +		return imsm_platform_test(hba);
>  
> -	/* it's static data so we only need to read it once */
> -	if (populated_orom[hba_id]) {
> -		dprintf("OROM CAP: %p, pid: %d pop: %d\n",
> -			&imsm_orom[hba_id], (int) getpid(), populated_orom[hba_id]);
> -		return &imsm_orom[hba_id];
> -	}
> -	if (check_env("IMSM_TEST_OROM")) {
> -		dprintf("OROM CAP: %p,  pid: %d pop: %d\n",
> -			&imsm_orom[hba_id], (int) getpid(), populated_orom[hba_id]);
> -		return imsm_platform_test(hba_id, &populated_orom[hba_id], &imsm_orom[hba_id]);
> -	}
>  	/* return empty OROM capabilities in EFI test mode */
> -	if (check_env("IMSM_TEST_AHCI_EFI") ||
> -	    check_env("IMSM_TEST_SCU_EFI"))
> +	if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
>  		return NULL;
>  
>  	find_intel_devices();
> @@ -325,9 +385,7 @@ static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
>  	scan_adapter_roms(scan);
>  	probe_roms_exit();
>  
> -	if (populated_orom[hba_id])
> -		return &imsm_orom[hba_id];
> -	return NULL;
> +	return get_orom_by_device_id(hba->dev_id);
>  }
>  
>  #define GUID_STR_MAX	37  /* according to GUID format:
> @@ -347,9 +405,7 @@ static const struct imsm_orom *find_imsm_hba_orom(enum sys_dev_type hba_id)
>  #define VENDOR_GUID \
>  	EFI_GUID(0x193dfefa, 0xa445, 0x4302, 0x99, 0xd8, 0xef, 0x3a, 0xad, 0x1a, 0x04, 0xc6)
>  
> -int populated_efi[SYS_DEV_MAX] = { 0, 0 };
> -
> -static struct imsm_orom imsm_efi[SYS_DEV_MAX];
> +#define PCI_CLASS_RAID_CNTRL 0x010400
>  
>  int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
>  {
> @@ -395,54 +451,40 @@ int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struc
>  	return 0;
>  }
>  
> -const struct imsm_orom *find_imsm_efi(enum sys_dev_type hba_id)
> +const struct imsm_orom *find_imsm_efi(struct sys_dev *hba)
>  {
> -	if (hba_id >= SYS_DEV_MAX)
> -		return NULL;
> +	struct imsm_orom orom;
> +	const struct imsm_orom *ret;
>  
> -	dprintf("EFI CAP: %p,  pid: %d pop: %d\n",
> -		&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
> +	if (check_env("IMSM_TEST_AHCI_EFI") || check_env("IMSM_TEST_SCU_EFI"))
> +		return imsm_platform_test(hba);
>  
> -	/* it's static data so we only need to read it once */
> -	if (populated_efi[hba_id]) {
> -		dprintf("EFI CAP: %p, pid: %d pop: %d\n",
> -			&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
> -		return &imsm_efi[hba_id];
> -	}
> -	if (check_env("IMSM_TEST_AHCI_EFI") ||
> -	    check_env("IMSM_TEST_SCU_EFI")) {
> -		dprintf("OROM CAP: %p,  pid: %d pop: %d\n",
> -			&imsm_efi[hba_id], (int) getpid(), populated_efi[hba_id]);
> -		return imsm_platform_test(hba_id, &populated_efi[hba_id], &imsm_efi[hba_id]);
> -	}
>  	/* OROM test is set, return that there is no EFI capabilities */
>  	if (check_env("IMSM_TEST_OROM"))
>  		return NULL;
>  
> -	if (read_efi_variable(&imsm_efi[hba_id], sizeof(imsm_efi[0]), hba_id == SYS_DEV_SAS ? SCU_PROP : AHCI_PROP, VENDOR_GUID)) {
> -		populated_efi[hba_id] = 0;
> +	if (hba->type == SYS_DEV_SATA && hba->class != PCI_CLASS_RAID_CNTRL)
>  		return NULL;
> -	}
>  
> -	populated_efi[hba_id] = 1;
> -	return &imsm_efi[hba_id];
> -}
> +	if (read_efi_variable(&orom, sizeof(orom), hba->type == SYS_DEV_SAS ? SCU_PROP : AHCI_PROP, VENDOR_GUID))
> +		return NULL;
>  
> -/*
> - * backward interface compatibility
> - */
> -const struct imsm_orom *find_imsm_orom(void)
> -{
> -	return find_imsm_hba_orom(SYS_DEV_SATA);
> +	ret = add_orom(&orom);
> +	add_orom_device_id(ret, hba->dev_id);
> +
> +	return ret;
>  }
>  
> -const struct imsm_orom *find_imsm_capability(enum sys_dev_type hba_id)
> +const struct imsm_orom *find_imsm_capability(struct sys_dev *hba)
>  {
> -	const struct imsm_orom *cap=NULL;
> +	const struct imsm_orom *cap = get_orom_by_device_id(hba->dev_id);
> +
> +	if (cap)
> +		return cap;
>  
> -	if ((cap = find_imsm_efi(hba_id)) != NULL)
> +	if ((cap = find_imsm_efi(hba)) != NULL)
>  		return cap;
> -	if ((cap = find_imsm_hba_orom(hba_id)) != NULL)
> +	if ((cap = find_imsm_hba_orom(hba)) != NULL)
>  		return cap;
>  	return NULL;
>  }
> diff --git a/platform-intel.h b/platform-intel.h
> index 8226be3..e41f386 100644
> --- a/platform-intel.h
> +++ b/platform-intel.h
> @@ -22,6 +22,7 @@
>  /* The IMSM Capability (IMSM AHCI and ISCU OROM/EFI variable) Version Table definition */
>  struct imsm_orom {
>  	__u8 signature[4];
> +	#define IMSM_OROM_SIGNATURE "$VER"
>  	__u8 table_ver_major; /* Currently 2 (can change with future revs) */
>  	__u8 table_ver_minor; /* Currently 2 (can change with future revs) */
>  	__u16 major_ver; /* Example: 8 as in 8.6.0.1020 */
> @@ -180,6 +181,7 @@ struct sys_dev {
>  	char *path;
>  	char *pci_id;
>  	__u16  dev_id;
> +	__u32  class;
>  	struct sys_dev *next;
>  };
>  
> @@ -201,10 +203,11 @@ static inline char *guid_str(char *buf, struct efi_guid guid)
>  char *diskfd_to_devpath(int fd);
>  struct sys_dev *find_driver_devices(const char *bus, const char *driver);
>  struct sys_dev *find_intel_devices(void);
> -const struct imsm_orom *find_imsm_capability(enum sys_dev_type hba_id);
> +const struct imsm_orom *find_imsm_capability(struct sys_dev *hba);
>  const struct imsm_orom *find_imsm_orom(void);
>  int disk_attached_to_hba(int fd, const char *hba_path);
>  int devt_attached_to_hba(dev_t dev, const char *hba_path);
>  char *devt_to_devpath(dev_t dev);
>  int path_attached_to_hba(const char *disk_path, const char *hba_path);
>  const char *get_sys_dev_type(enum sys_dev_type);
> +const struct imsm_orom *get_orom_by_device_id(__u16 device_id);
> diff --git a/super-intel.c b/super-intel.c
> index e28ac7d..dabf011 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -555,11 +555,26 @@ static int attach_hba_to_super(struct intel_super *super, struct sys_dev *device
>  	if (super->hba == NULL) {
>  		super->hba = alloc_intel_hba(device);
>  		return 1;
> -	} else
> -		/* IMSM metadata disallows to attach disks to multiple
> -		 * controllers.
> -		 */
> +	}
> +
> +	hba = super->hba;
> +	/* Intel metadata allows for all disks attached to the same type HBA.
> +	 * Do not sypport odf HBA types mixing
> +	 */
> +	if (device->type != hba->type)
> +		return 2;
> +
> +	/* Multiple same type HBAs can be used if they share the same OROM */
> +	const struct imsm_orom *device_orom = get_orom_by_device_id(device->dev_id);
> +
> +	if (device_orom != super->orom)
>  		return 2;
> +
> +	while (hba->next)
> +		hba = hba->next;
> +
> +	hba->next = alloc_intel_hba(device);
> +	return 1;
>  }
>  
>  static struct sys_dev* find_disk_attached_hba(int fd, const char *devname)
> @@ -1886,13 +1901,12 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
>  		if (!list)
>  			return 2;
>  		for (hba = list; hba; hba = hba->next) {
> -			orom = find_imsm_capability(hba->type);
> -			if (!orom) {
> -				result = 2;
> +			if (find_imsm_capability(hba)) {
> +				result = 0;
>  				break;
>  			}
>  			else
> -				result = 0;
> +				result = 2;
>  		}
>  		return result;
>  	}
> @@ -1909,7 +1923,7 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
>  	for (hba = list; hba; hba = hba->next) {
>  		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
>  			continue;
> -		orom = find_imsm_capability(hba->type);
> +		orom = find_imsm_capability(hba);
>  		if (!orom)
>  			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
>  				hba->path, get_sys_dev_type(hba->type));
> @@ -1954,7 +1968,7 @@ static int export_detail_platform_imsm(int verbose, char *controller_path)
>  	for (hba = list; hba; hba = hba->next) {
>  		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
>  			continue;
> -		orom = find_imsm_capability(hba->type);
> +		orom = find_imsm_capability(hba);
>  		if (!orom) {
>  			if (verbose > 0)
>  				pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICE_UNDER_%s\n",hba->path);
> @@ -3087,13 +3101,18 @@ static int compare_super_imsm(struct supertype *st, struct supertype *tst)
>  	 * use the same Intel hba
>  	 * If not on Intel hba at all, allow anything.
>  	 */
> -	if (!check_env("IMSM_NO_PLATFORM")) {
> -		if (first->hba && sec->hba &&
> -		    strcmp(first->hba->path, sec->hba->path) != 0)  {
> +	if (!check_env("IMSM_NO_PLATFORM") && first->hba && sec->hba) {
> +		if (first->hba->type != sec->hba->type) {
> +			fprintf(stderr,
> +				"HBAs of devices do not match %s != %s\n",
> +				get_sys_dev_type(first->hba->type),
> +				get_sys_dev_type(sec->hba->type));
> +			return 3;
> +		}
> +		if (first->orom != sec->orom) {
>  			fprintf(stderr,
> -				"HBAs of devices does not match %s != %s\n",
> -				first->hba ? first->hba->path : NULL,
> -				sec->hba ? sec->hba->path : NULL);
> +				"HBAs of devices do not match %s != %s\n",
> +				first->hba->pci_id, sec->hba->pci_id);
>  			return 3;
>  		}
>  	}
> @@ -3832,14 +3851,13 @@ static int find_intel_hba_capability(int fd, struct intel_super *super, char *de
>  					fprintf(stderr, ", ");
>  				hba = hba->next;
>  			}
> -
> -			fprintf(stderr, ").\n");
> -			cont_err("Mixing devices attached to multiple controllers "
> -				 "is not allowed.\n");
> +			fprintf(stderr, ").\n"
> +				"    Mixing devices attached to different controllers "
> +				"is not allowed.\n");
>  		}
>  		return 2;
>  	}
> -	super->orom = find_imsm_capability(hba_name->type);
> +	super->orom = find_imsm_capability(hba_name);
>  	if (!super->orom)
>  		return 3;
>  	return 0;
> @@ -9061,32 +9079,68 @@ int open_backup_targets(struct mdinfo *info, int raid_disks, int *raid_fds,
>   ******************************************************************************/
>  int validate_container_imsm(struct mdinfo *info)
>  {
> -	if (!check_env("IMSM_NO_PLATFORM")) {
> -		struct sys_dev *idev;
> -		struct mdinfo *dev;
> -		char *hba_path = NULL;
> -		char *dev_path = devt_to_devpath(makedev(info->disk.major,
> -										info->disk.minor));
> +	if (check_env("IMSM_NO_PLATFORM"))
> +		return 0;
>  
> -		for (idev = find_intel_devices(); idev; idev = idev->next) {
> -			if (strstr(dev_path, idev->path)) {
> -				hba_path = idev->path;
> -				break;
> -			}
> +	struct sys_dev *idev;
> +	struct sys_dev *hba = NULL;
> +	struct sys_dev *intel_devices = find_intel_devices();
> +	char *dev_path = devt_to_devpath(makedev(info->disk.major,
> +									info->disk.minor));
> +
> +	for (idev = intel_devices; idev; idev = idev->next) {
> +		if (dev_path && strstr(dev_path, idev->path)) {
> +			hba = idev;
> +			break;
>  		}
> +	}
> +	if (dev_path)
>  		free(dev_path);
>  
> -		if (hba_path) {
> -			for (dev = info->next; dev; dev = dev->next) {
> -				if (!devt_attached_to_hba(makedev(dev->disk.major,
> -						dev->disk.minor), hba_path)) {
> -					pr_err("WARNING - IMSM container assembled with disks under different HBAs!\n"
> -						"       This operation is not supported and can lead to data loss.\n");
> -					return 1;
> -				}
> +	if (!hba) {
> +		pr_err("WARNING - Cannot detect HBA for device %s!\n",
> +				devid2kname(makedev(info->disk.major, info->disk.minor)));
> +		return 1;
> +	}
> +
> +	const struct imsm_orom *orom = get_orom_by_device_id(hba->dev_id);
> +	struct mdinfo *dev;
> +
> +	for (dev = info->next; dev; dev = dev->next) {
> +		dev_path = devt_to_devpath(makedev(dev->disk.major, dev->disk.minor));
> +
> +		struct sys_dev *hba2 = NULL;
> +		for (idev = intel_devices; idev; idev = idev->next) {
> +			if (dev_path && strstr(dev_path, idev->path)) {
> +				hba2 = idev;
> +				break;
>  			}
>  		}
> +		if (dev_path)
> +			free(dev_path);
> +
> +		const struct imsm_orom *orom2 = hba2 == NULL ? NULL :
> +				get_orom_by_device_id(hba2->dev_id);
> +
> +		if (hba2 && hba->type != hba2->type) {
> +			pr_err("WARNING - HBAs of devices do not match %s != %s\n",
> +				get_sys_dev_type(hba->type), get_sys_dev_type(hba2->type));
> +			return 1;
> +		}
> +
> +		if (orom != orom2) {
> +			pr_err("WARNING - IMSM container assembled with disks under different HBAs!\n"
> +				"       This operation is not supported and can lead to data loss.\n");
> +			return 1;
> +		}
> +
> +		if (!orom) {
> +			pr_err("WARNING - IMSM container assembled with disks under HBAs without IMSM platform support!\n"
> +				"       This operation is not supported and can lead to data loss.\n");
> +			return 1;
> +		}
>  	}
> +
>  	return 0;
>  }
>  #ifndef MDASSEMBLE


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: mdadm creates corrupt superblock
From: NeilBrown @ 2014-11-19 21:47 UTC (permalink / raw)
  To: Hans Kraus; +Cc: Linux-RAID
In-Reply-To: <546C68F9.7070307@hanswkraus.com>

[-- Attachment #1: Type: text/plain, Size: 10150 bytes --]

On Wed, 19 Nov 2014 10:55:05 +0100 Hans Kraus <hans@hanswkraus.com> wrote:

> Hi,
> 
> I think I found the problem. 'badblocks' reports sector 64 of the HD as
> errornous. But the HD neither reports a read error nor remaps that
> sector.
> 
> Is it possible to parametrize mdadm that it doesn't use that sector?

I'm surprised that md is using sector 64.
The superblock is at sectors 8,9
The bitmap at 16..24, or something like that
The data is at 262144 onwards.

But that are offsets in sdb1, maybe the '64' is a sector number in 'sdb'.

Where does 'sdb1' start in 'sdb'??

NeilBrown


> 
> Regards, Hans
> 
> Am 29.10.2014 22:11, schrieb Hans Kraus:
> > Hi Neil,
> >
> > many thanks for your response. I rebooted the machine and got new drive
> > assignments. The drive in question is now sdc1. mdadm gave an error with
> > the dump, results below (nothing to report by dmesg) [by the way, I
> > wasn'nt even able to create an array on that drive with
> > 'mdadm --create --level=1 -n 2 /dev/sdc1 missing', same error(s)]:
> > -------------------------------------------------------------------------
> > root@nashorn:~# mdadm -E /dev/sdb1
> > /dev/sdb1:
> >            Magic : a92b4efc
> >          Version : 1.2
> >      Feature Map : 0x1
> >       Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >             Name : nashorn:126  (local to host nashorn)
> >    Creation Time : Mon Oct 27 15:58:38 2014
> >       Raid Level : raid1
> >     Raid Devices : 2
> >
> >   Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >       Array Size : 976630488 (931.39 GiB 1000.07 GB)
> >      Data Offset : 262144 sectors
> >     Super Offset : 8 sectors
> >     Unused Space : before=262056 sectors, after=0 sectors
> >            State : clean
> >      Device UUID : f7a91c46:e4cda0c0:1b770fd7:876634f8
> >
> > Internal Bitmap : 8 sectors from superblock
> >      Update Time : Wed Oct 29 18:42:16 2014
> >    Bad Block Log : 512 entries available at offset 72 sectors
> >         Checksum : 939b22d1 - correct
> >           Events : 3325
> >
> >
> >     Device Role : Active device 0
> >     Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
> > root@nashorn:~#
> > -------------------------------------------------------------------------
> > root@nashorn:~# mdadm -E /dev/sdh1
> > /dev/sdh1:
> >            Magic : a92b4efc
> >          Version : 1.2
> >      Feature Map : 0x1
> >       Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >             Name : nashorn:126  (local to host nashorn)
> >    Creation Time : Mon Oct 27 15:58:38 2014
> >       Raid Level : raid1
> >     Raid Devices : 2
> >
> >   Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >       Array Size : 976630488 (931.39 GiB 1000.07 GB)
> >      Data Offset : 262144 sectors
> >     Super Offset : 8 sectors
> >     Unused Space : before=262046 sectors, after=0 sectors
> >            State : clean
> >      Device UUID : 089d1d0e:4d5ca222:dd716acb:0813815c
> >
> > Internal Bitmap : 8 sectors from superblock
> >      Update Time : Wed Oct 29 18:42:16 2014
> >    Bad Block Log : 512 entries available at offset 82 sectors
> >         Checksum : 16454b96 - correct
> >           Events : 3325
> >
> >
> >     Device Role : Active device 1
> >     Array State : AA ('A' == active, '.' == missing, 'R' == replacing)
> > root@nashorn:~#
> > -------------------------------------------------------------------------
> > root@nashorn:~# mdadm -E /dev/sdc1
> > /dev/sdc1:
> >            Magic : a92b4efc
> >          Version : 1.2
> >      Feature Map : 0x1
> >       Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >             Name : nashorn:126  (local to host nashorn)
> >    Creation Time : Mon Oct 27 15:58:38 2014
> >       Raid Level : raid1
> >     Raid Devices : 2
> >
> >   Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >       Array Size : 0
> >    Used Dev Size : 0
> >      Data Offset : 262144 sectors
> >     Super Offset : 8 sectors
> >     Unused Space : before=262056 sectors, after=1953260976 sectors
> >            State : clean
> >      Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
> >
> > Internal Bitmap : 8 sectors from superblock
> >      Update Time : Tue Oct 28 11:53:49 2014
> >    Bad Block Log : 512 entries available at offset 72 sectors
> >         Checksum : 6f02d040 - expected 891eedff
> >           Events : 0
> >
> >
> >     Device Role : spare
> >     Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
> > -------------------------------------------------------------------------
> > root@nashorn:~# mkdir -p /tmp/dump
> > root@nashorn:~# mdadm --dump /tmp/dump /dev/sdc1
> > mdadm: Failed to copy metadata from /dev/sdc1 to /tmp/dump/sdc1
> > -------------------------------------------------------------------------
> > Kind regards, Hans
> >
> > Am 28.10.2014 22:00, schrieb NeilBrown:
> >> On Tue, 28 Oct 2014 19:06:20 +0100 Hans Kraus <hans@hanswkraus.com>
> >> wrote:
> >>
> >>> Hi,
> >>>
> >>> I debugged my problem a bit more: it seems that mdadm creates a defect
> >>> superblock. That's repeatedly happening, but only for one drive. I
> >>> copied zeros to that drive (via ddrescue /dev/zero ...) and the drive
> >>> looks OK. The info I got:
> >>>
> >>
> >> and you send me the metadata of all your devices please?
> >>
> >>     mkdir /tmp/dump
> >>     mdadm --dump /tmp/dump /dev/sde1
> >>     tar czvf /tmp/dump.tgz /tmp/dump
> >>
> >> and then send /tmp/dump.tgz
> >>
> >> Thanks.
> >>
> >> NeilBrown
> >>
> >> =================================================================================================================================================================================================
> >>
> >>> root@nashorn:/home/kraush/work/smartctrl# mdadm --zero-superblock
> >>> /dev/sde1
> >>> mdadm: Unrecognised md component device - /dev/sde1
> >>> root@nashorn:/home/kraush/work/smartctrl# mdadm --add /dev/md126
> >>> /dev/sde1
> >>> mdadm: add new device failed for /dev/sde1 as 4: Invalid argument
> >>> root@nashorn:/home/kraush/work/smartctrl# mdadm -E /dev/sde1
> >>> /dev/sde1:
> >>>             Magic : a92b4efc
> >>>           Version : 1.2
> >>>       Feature Map : 0x1
> >>>        Array UUID : e7caa5d1:b33fd2a5:7782fb0c:9d8d9d5b
> >>>              Name : nashorn:126  (local to host nashorn)
> >>>     Creation Time : Mon Oct 27 15:58:38 2014
> >>>        Raid Level : raid1
> >>>      Raid Devices : 2
> >>>
> >>>    Avail Dev Size : 1953260976 (931.39 GiB 1000.07 GB)
> >>>        Array Size : 0
> >>>     Used Dev Size : 0
> >>>       Data Offset : 262144 sectors
> >>>      Super Offset : 8 sectors
> >>>      Unused Space : before=262056 sectors, after=1953260976 sectors
> >>>             State : clean
> >>>       Device UUID : 4d1420c6:79978477:e86412b1:24beebfc
> >>>
> >>> Internal Bitmap : 8 sectors from superblock
> >>>       Update Time : Tue Oct 28 11:53:49 2014
> >>>     Bad Block Log : 512 entries available at offset 72 sectors
> >>>          Checksum : 6f02d040 - expected 891eedff
> >>>            Events : 0
> >>>
> >>>
> >>>      Device Role : spare
> >>>      Array State : RR ('A' == active, '.' == missing, 'R' == replacing)
> >>> root@nashorn:/home/kraush/work/smartctrl#
> >>> -----------------------------------------------------------------------------------------------------------
> >>>
> >>> root@nashorn:/home/kraush/work/smartctrl# cat /proc/mdstat
> >>> Personalities : [raid1] [raid6] [raid5] [raid4]
> >>> md127 : active (auto-read-only) raid6 sda1[0] sdk1[6](S) sdl2[7](S)
> >>> sdh1[5] sdg1[4] sdf1[3] sdc1[2] sdb1[1]
> >>>         1953017856 blocks super 1.2 level 6, 512k chunk, algorithm 2
> >>> [6/6] [UUUUUU]
> >>>         bitmap: 0/4 pages [0KB], 65536KB chunk
> >>>
> >>> md10 : active (auto-read-only) raid1 sdi2[0] sdl3[1]
> >>>         87833408 blocks super 1.2 [2/2] [UU]
> >>>         bitmap: 0/1 pages [0KB], 65536KB chunk
> >>>
> >>> md126 : active raid1 sdd1[3] sdj1[2]
> >>>         488254464 blocks super 1.2 [2/2] [UU]
> >>>         bitmap: 0/4 pages [0KB], 65536KB chunk
> >>>
> >>> md0 : active raid1 sdl1[4] sdd2[3] sdi1[5]
> >>>         156157824 blocks super 1.2 [3/1] [U__]
> >>>         [===================>.]  recovery = 95.6% (149290048/156157824)
> >>> finish=8.4min speed=13496K/sec
> >>>         bitmap: 1/2 pages [4KB], 65536KB chunk
> >>>
> >>> unused devices: <none>
> >>> root@nashorn:/home/kraush/work/smartctrl#
> >>> -----------------------------------------------------------------------------------------------------------
> >>>
> >>> dmesg:
> >>> [33299.387382] md: invalid superblock checksum on sde1
> >>> [33299.387385] md: sde1 does not have a valid v1.2 superblock, not
> >>> importing!
> >>> [33299.387408] md: md_import_device returned -22
> >>> root@nashorn:/home/kraush/work/smartctrl#
> >>> -----------------------------------------------------------------------------------------------------------
> >>>
> >>> root@nashorn:/home/kraush/work/mdadm# mdadm --version
> >>> mdadm - v3.3-161-gfed12d4 - 21st August 2014
> >>> =================================================================================================================================================================================================
> >>>
> >>>
> >>> Kind regards, Hans
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* [PATCH 1/1] dm: Deletion of unnecessary checks before two function calls
From: SF Markus Elfring @ 2014-11-19 20:55 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Neil Brown, dm-devel, linux-raid
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 19 Nov 2014 21:48:10 +0100

The functions dm_table_destroy() and vfree() perform also input
parameter validation. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/dm-ioctl.c           |  3 +--
 drivers/md/dm-snap-persistent.c | 12 ++++--------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 5152142..0b925a4 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1050,8 +1050,7 @@ static int do_resume(struct dm_ioctl *param)
 	 * Since dm_swap_table synchronizes RCU, nobody should be in
 	 * read-side critical section already.
 	 */
-	if (old_map)
-		dm_table_destroy(old_map);
+	dm_table_destroy(old_map);
 
 	if (!r)
 		__dev_status(md, param);
diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
index d6e8817..4b29bac 100644
--- a/drivers/md/dm-snap-persistent.c
+++ b/drivers/md/dm-snap-persistent.c
@@ -200,16 +200,13 @@ err_area:
 
 static void free_area(struct pstore *ps)
 {
-	if (ps->area)
-		vfree(ps->area);
+	vfree(ps->area);
 	ps->area = NULL;
 
-	if (ps->zero_area)
-		vfree(ps->zero_area);
+	vfree(ps->zero_area);
 	ps->zero_area = NULL;
 
-	if (ps->header_area)
-		vfree(ps->header_area);
+	vfree(ps->header_area);
 	ps->header_area = NULL;
 }
 
@@ -605,8 +602,7 @@ static void persistent_dtr(struct dm_exception_store *store)
 	free_area(ps);
 
 	/* Allocated in persistent_read_metadata */
-	if (ps->callbacks)
-		vfree(ps->callbacks);
+	vfree(ps->callbacks);
 
 	kfree(ps);
 }
-- 
2.1.3

^ permalink raw reply related

* [PATCH 1/1] bcache: Deletion of an unnecessary check before the function call "kobject_put"
From: SF Markus Elfring @ 2014-11-19 20:34 UTC (permalink / raw)
  To: Kent Overstreet, Neil Brown, linux-bcache
  Cc: linux-raid, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 19 Nov 2014 21:25:39 +0100

The kobject_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/bcache/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index d4713d0..1443c55 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2076,8 +2076,7 @@ static void bcache_exit(void)
 {
 	bch_debug_exit();
 	bch_request_exit();
-	if (bcache_kobj)
-		kobject_put(bcache_kobj);
+	kobject_put(bcache_kobj);
 	if (bcache_wq)
 		destroy_workqueue(bcache_wq);
 	if (bcache_major)
-- 
2.1.3

^ permalink raw reply related

* Re: Re-assemble RAID6 from SW to HW RAID
From: Mathias Burén @ 2014-11-19 19:06 UTC (permalink / raw)
  To: Drew; +Cc: Phil Turmel, Linux-RAID
In-Reply-To: <CACJz6QunR70Rom7ht2L5f-FykciVtawXg1UkYxS6VDMkejYfnA@mail.gmail.com>

On 18 November 2014 20:09, Drew <drew.kay@gmail.com> wrote:
> On Tue, Nov 18, 2014 at 5:34 AM, Phil Turmel <philip@turmel.org> wrote:
>>
>> You should look if your MegaRAID has a 'JBOD' mode for the drives.
>
> Some LSI controllers, 1068e as well as 9(2,4,6)00 series controllers,
> can be re-flashed to make them plain HBA's
>
>
> --
> Drew
>
> "Nothing in life is to be feared. It is only to be understood."
> --Marie Curie


Yeah I tried but it didn't appear to work. Oh well it looks like I'm
rebuilding the array from scratch!

Regards
Mathias

^ permalink raw reply

* [PATCH 5/5] imsm: use efivarfs interface for reading UEFI variables
From: Artur Paszkiewicz @ 2014-11-19 12:53 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, pawel.baldysiak, Artur Paszkiewicz
In-Reply-To: <1416401610-16209-1-git-send-email-artur.paszkiewicz@intel.com>

Read UEFI variables using the new efivarfs interface, fallback to
sysfs-efivars if that fails.

Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
 platform-intel.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/platform-intel.c b/platform-intel.c
index 54ef37f..586a2f6 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -416,6 +416,7 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
 
 #define SYS_EFI_VAR_PATH "/sys/firmware/efi/vars"
+#define SYS_EFIVARS_PATH "/sys/firmware/efi/efivars"
 #define SCU_PROP "RstScuV"
 #define AHCI_PROP "RstSataV"
 #define AHCI_SSATA_PROP "RstsSatV"
@@ -426,10 +427,44 @@ static const struct imsm_orom *find_imsm_hba_orom(struct sys_dev *hba)
 
 #define PCI_CLASS_RAID_CNTRL 0x010400
 
-int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
+static int read_efi_var(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
 {
 	char path[PATH_MAX];
 	char buf[GUID_STR_MAX];
+	int fd;
+	ssize_t n;
+
+	snprintf(path, PATH_MAX, "%s/%s-%s", SYS_EFIVARS_PATH, variable_name, guid_str(buf, guid));
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return 1;
+
+	/* read the variable attributes and ignore it */
+	n = read(fd, buf, sizeof(__u32));
+	if (n < 0) {
+		close(fd);
+		return 1;
+	}
+
+	/* read the variable data */
+	n = read(fd, buffer, buf_size);
+	close(fd);
+	if (n < buf_size)
+		return 1;
+
+	return 0;
+}
+
+static int read_efi_variable(void *buffer, ssize_t buf_size, char *variable_name, struct efi_guid guid)
+{
+	/* Try to read the variable using the new efivarfs interface first.
+	 * If that fails, fall back to the old sysfs-efivars interface. */
+	if (!read_efi_var(buffer, buf_size, variable_name, guid))
+		return 0;
+
+	char path[PATH_MAX];
+	char buf[GUID_STR_MAX];
 	int dfd;
 	ssize_t n, var_data_len;
 
-- 
1.8.4.5


^ permalink raw reply related

* [PATCH 4/5] imsm: detail-platform improvements
From: Artur Paszkiewicz @ 2014-11-19 12:53 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, pawel.baldysiak, Artur Paszkiewicz
In-Reply-To: <1416401610-16209-1-git-send-email-artur.paszkiewicz@intel.com>

Print platform details per OROM, not per controller, differentiate
RST(e) platforms from legacy IMSM, print NVMe device paths, adjust port
printing to newer sysfs path.

Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
 platform-intel.c | 26 ++++++++++------
 platform-intel.h | 23 ++++++++++++++
 super-intel.c    | 93 ++++++++++++++++++++++++++++++++++++++------------------
 3 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/platform-intel.c b/platform-intel.c
index ae72827..54ef37f 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -134,6 +134,16 @@ struct sys_dev *find_driver_devices(const char *bus, const char *driver)
 static struct sys_dev *intel_devices=NULL;
 static time_t valid_time = 0;
 
+struct sys_dev *device_by_id(__u16 device_id)
+{
+	struct sys_dev *iter;
+
+	for (iter = intel_devices; iter != NULL; iter = iter->next)
+		if (iter->dev_id == device_id)
+			return iter;
+	return NULL;
+}
+
 static int devpath_to_ll(const char *dev_path, const char *entry, unsigned long long *val)
 {
 	char path[strlen(dev_path) + strlen(entry) + 2];
@@ -219,18 +229,13 @@ struct pciExpDataStructFormat {
 	__u16 devListOffset;
 } __attribute__ ((packed));
 
-struct devid_list {
-	__u16 devid;
-	struct devid_list *next;
-};
-
-struct orom_entry {
-	struct imsm_orom orom;
-	struct devid_list *devid_list;
-};
-
 static struct orom_entry oroms[SYS_DEV_MAX];
 
+const struct orom_entry *get_oroms(void)
+{
+	return (const struct orom_entry *)&oroms;
+}
+
 const struct imsm_orom *get_orom_by_device_id(__u16 dev_id)
 {
 	int i;
@@ -529,6 +534,7 @@ const struct imsm_orom *find_imsm_nvme(struct sys_dev *hba)
 			.vpa = IMSM_OROM_VOLUMES_PER_ARRAY,
 			.vphba = IMSM_OROM_TOTAL_DISKS_NVME / 2 * IMSM_OROM_VOLUMES_PER_ARRAY,
 			.attr = IMSM_OROM_ATTR_2TB | IMSM_OROM_ATTR_2TB_DISK,
+			.driver_features = IMSM_OROM_CAPABILITIES_EnterpriseSystem
 		};
 		nvme_orom = add_orom(&nvme_orom_compat);
 	}
diff --git a/platform-intel.h b/platform-intel.h
index 6b4ebd8..3e85d44 100644
--- a/platform-intel.h
+++ b/platform-intel.h
@@ -173,6 +173,17 @@ static inline int fls(int x)
 	return r;
 }
 
+static inline int imsm_orom_is_enterprise(const struct imsm_orom *orom)
+{
+	return !!(orom->driver_features & IMSM_OROM_CAPABILITIES_EnterpriseSystem);
+}
+
+static inline int imsm_orom_is_nvme(const struct imsm_orom *orom)
+{
+	return memcmp(orom->signature, IMSM_NVME_OROM_COMPAT_SIGNATURE,
+			sizeof(orom->signature)) == 0;
+}
+
 enum sys_dev_type {
 	SYS_DEV_UNKNOWN = 0,
 	SYS_DEV_SAS,
@@ -194,6 +205,16 @@ struct efi_guid {
 	__u8 b[16];
 };
 
+struct devid_list {
+	__u16 devid;
+	struct devid_list *next;
+};
+
+struct orom_entry {
+	struct imsm_orom orom;
+	struct devid_list *devid_list;
+};
+
 static inline char *guid_str(char *buf, struct efi_guid guid)
 {
 	sprintf(buf, "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
@@ -215,4 +236,6 @@ int devt_attached_to_hba(dev_t dev, const char *hba_path);
 char *devt_to_devpath(dev_t dev);
 int path_attached_to_hba(const char *disk_path, const char *hba_path);
 const char *get_sys_dev_type(enum sys_dev_type);
+const struct orom_entry * get_oroms(void);
 const struct imsm_orom *get_orom_by_device_id(__u16 device_id);
+struct sys_dev *device_by_id(__u16 device_id);
diff --git a/super-intel.c b/super-intel.c
index d2ee1c6..4c53019 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1709,7 +1709,8 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
 			break;
 		}
 		*c = '\0';
-		if (sscanf(&path[hba_len], "host%d", &port) == 1)
+		if ((sscanf(&path[hba_len], "ata%d", &port) == 1) ||
+		   ((sscanf(&path[hba_len], "host%d", &port) == 1)))
 			port -= host_base;
 		else {
 			if (verbose > 0) {
@@ -1768,6 +1769,8 @@ static void print_found_intel_controllers(struct sys_dev *elem)
 			fprintf(stderr, "SATA ");
 		else if (elem->type == SYS_DEV_SAS)
 			fprintf(stderr, "SAS ");
+		else if (elem->type == SYS_DEV_NVME)
+			fprintf(stderr, "NVMe ");
 		fprintf(stderr, "RAID controller");
 		if (elem->pci_id)
 			fprintf(stderr, " at %s", elem->pci_id);
@@ -1789,7 +1792,8 @@ static int ahci_get_port_count(const char *hba_path, int *port_count)
 	for (ent = readdir(dir); ent; ent = readdir(dir)) {
 		int host;
 
-		if (sscanf(ent->d_name, "host%d", &host) != 1)
+		if ((sscanf(ent->d_name, "ata%d", &host) != 1) &&
+		   ((sscanf(ent->d_name, "host%d", &host) != 1)))
 			continue;
 		if (*port_count == 0)
 			host_base = host;
@@ -1805,9 +1809,15 @@ static int ahci_get_port_count(const char *hba_path, int *port_count)
 
 static void print_imsm_capability(const struct imsm_orom *orom)
 {
-	printf("       Platform : Intel(R) Matrix Storage Manager\n");
-	printf("        Version : %d.%d.%d.%d\n", orom->major_ver, orom->minor_ver,
-	       orom->hotfix_ver, orom->build);
+	printf("       Platform : Intel(R) ");
+	if (orom->capabilities == 0 && orom->driver_features == 0)
+		printf("Matrix Storage Manager\n");
+	else
+		printf("Rapid Storage Technology%s\n",
+			imsm_orom_is_enterprise(orom) ? " enterprise" : "");
+	if (orom->major_ver || orom->minor_ver || orom->hotfix_ver || orom->build)
+		printf("        Version : %d.%d.%d.%d\n", orom->major_ver,
+				orom->minor_ver, orom->hotfix_ver, orom->build);
 	printf("    RAID Levels :%s%s%s%s%s\n",
 	       imsm_orom_has_raid0(orom) ? " raid0" : "",
 	       imsm_orom_has_raid1(orom) ? " raid1" : "",
@@ -1836,16 +1846,18 @@ static void print_imsm_capability(const struct imsm_orom *orom)
 	printf("      2TB disks :%s supported\n",
 	       (orom->attr & IMSM_OROM_ATTR_2TB_DISK)?"":" not");
 	printf("      Max Disks : %d\n", orom->tds);
-	printf("    Max Volumes : %d per array, %d per controller\n",
-	       orom->vpa, orom->vphba);
+	printf("    Max Volumes : %d per array, %d per %s\n",
+	       orom->vpa, orom->vphba,
+	       imsm_orom_is_nvme(orom) ? "platform" : "controller");
 	return;
 }
 
 static void print_imsm_capability_export(const struct imsm_orom *orom)
 {
 	printf("MD_FIRMWARE_TYPE=imsm\n");
-	printf("IMSM_VERSION=%d.%d.%d.%d\n",orom->major_ver, orom->minor_ver,
-			orom->hotfix_ver, orom->build);
+	if (orom->major_ver || orom->minor_ver || orom->hotfix_ver || orom->build)
+		printf("IMSM_VERSION=%d.%d.%d.%d\n", orom->major_ver, orom->minor_ver,
+				orom->hotfix_ver, orom->build);
 	printf("IMSM_SUPPORTED_RAID_LEVELS=%s%s%s%s%s\n",
 			imsm_orom_has_raid0(orom) ? "raid0 " : "",
 			imsm_orom_has_raid1(orom) ? "raid1 " : "",
@@ -1889,7 +1901,6 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
 	 * platform capabilities.  If raid support is disabled in the BIOS the
 	 * option-rom capability structure will not be available.
 	 */
-	const struct imsm_orom *orom;
 	struct sys_dev *list, *hba;
 	int host_base = 0;
 	int port_count = 0;
@@ -1922,15 +1933,42 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
 		print_found_intel_controllers(list);
 
 	for (hba = list; hba; hba = hba->next) {
-		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
+		if (controller_path && (compare_paths(hba->path, controller_path) != 0))
 			continue;
-		orom = find_imsm_capability(hba);
-		if (!orom)
+		if (!find_imsm_capability(hba)) {
 			pr_err("imsm capabilities not found for controller: %s (type %s)\n",
 				hba->path, get_sys_dev_type(hba->type));
-		else {
-			result = 0;
-			print_imsm_capability(orom);
+			continue;
+		}
+		result = 0;
+	}
+
+	if (controller_path && result == 1) {
+		pr_err("no active Intel(R) RAID controller found under %s\n",
+				controller_path);
+		return result;
+	}
+
+	const struct orom_entry *oroms = get_oroms();
+	int i;
+
+	for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++) {
+		print_imsm_capability(&oroms[i].orom);
+
+		if (imsm_orom_is_nvme(&oroms[i].orom)) {
+			for (hba = list; hba; hba = hba->next) {
+				if (hba->type == SYS_DEV_NVME)
+					printf("    NVMe Device : %s\n", hba->path);
+			}
+			continue;
+		}
+
+		struct devid_list *devid;
+		for (devid = oroms[i].devid_list; devid; devid = devid->next) {
+			hba = device_by_id(devid->devid);
+			if (!hba)
+				continue;
+
 			printf(" I/O Controller : %s (%s)\n",
 				hba->path, get_sys_dev_type(hba->type));
 			if (hba->type == SYS_DEV_SATA) {
@@ -1943,18 +1981,14 @@ static int detail_platform_imsm(int verbose, int enumerate_only, char *controlle
 				}
 			}
 		}
+		printf("\n");
 	}
 
-	if (controller_path && result == 1)
-		pr_err("no active Intel(R) RAID "
-				"controller found under %s\n",controller_path);
-
 	return result;
 }
 
 static int export_detail_platform_imsm(int verbose, char *controller_path)
 {
-	const struct imsm_orom *orom;
 	struct sys_dev *list, *hba;
 	int result=1;
 
@@ -1969,17 +2003,18 @@ static int export_detail_platform_imsm(int verbose, char *controller_path)
 	for (hba = list; hba; hba = hba->next) {
 		if (controller_path && (compare_paths(hba->path,controller_path) != 0))
 			continue;
-		orom = find_imsm_capability(hba);
-		if (!orom) {
-			if (verbose > 0)
-				pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICE_UNDER_%s\n",hba->path);
-		}
-		else {
-			print_imsm_capability_export(orom);
+		if (!find_imsm_capability(hba) && verbose > 0)
+			pr_err("IMSM_DETAIL_PLATFORM_ERROR=NO_IMSM_CAPABLE_DEVICE_UNDER_%s\n", hba->path);
+		else
 			result = 0;
-		}
 	}
 
+	const struct orom_entry *oroms = get_oroms();
+	int i;
+
+	for (i = 0; i < SYS_DEV_MAX && oroms[i].devid_list; i++)
+		print_imsm_capability_export(&oroms[i].orom);
+
 	return result;
 }
 
-- 
1.8.4.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox