Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: Request for assistance
From: o1bigtenor @ 2016-07-06 18:28 UTC (permalink / raw)
  To: Wols Lists; +Cc: Adam Goryachev, Linux-RAID
In-Reply-To: <577CFEC4.7040704@youngman.org.uk>

On Wed, Jul 6, 2016 at 7:51 AM, Wols Lists <antlists@youngman.org.uk> wrote:
> On 06/07/16 13:14, o1bigtenor wrote:
>> On Tue, Jul 5, 2016 at 8:55 PM, Adam Goryachev
>> <mailinglists@websitemanagers.com.au> wrote:
>>> On 06/07/16 10:13, o1bigtenor wrote:
>>>>
>>>> Greetings
>>>>
>>>> Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
>>>> gets significant lightning and also brownout (rural power) events.
>>>>
>> snip
snip
>>
>> So my array is back up - - - thank you very much for your assistance!!!
>>
> But why did they drop ... are you using desktop drives? I use Seagate
> Barracudas - NOT a particularly good idea. You should be using WD Red,
> Seagate NAS, or similar.

Sorry - - - this system is 4 1 TB WD Red drives
>
> "smartctl -x /dev/sdx" will give you an idea of what's going on. Search
> the list for "timeout error" for an idea of the grief you'll get if
> you're using desktop drives ...
>
> If smartctl says smart is disabled, enable it. When I do, my drive comes
> back (using the -x option again) saying "SCT Error Recovery not
> supported". This is a no-no for a decent raid drive. I think the other
> acronyms are ETL or TLS - either way you can control how the drive
> reports an error back to the OS. Which is why you need proper raid
> drives (the manufacturers have downgraded the firmware on desktop drives :-(
>
> You need to fix the WHY or it could easily happen again. And this could
> well be why ... (if you've had a problem on a desktop drive, it WILL
> happen again, and data loss is quite likely ... even if you recover the
> bulk of the drive).

My best understanding as to the why is - - dirty power - - - fixing that means
going off-grid. Expensive and not happening any time soon although I would
really like that.

As I do not understand the error messages in smartctl I add the following
(maybe someone would explain what they mean) :

smartctl -x /dev/sdf
smartctl 6.4 2014-10-07 r4002 [x86_64-linux-4.1.0-2-amd64] (local build)
Copyright (C) 2002-14, Bruce Allen, Christian Franke, www.smartmontools.org

=== START OF INFORMATION SECTION ===
Model Family:     Western Digital Red (AF)
Device Model:     WDC WD10EFRX-68FYTN0
Serial Number:    WD-WCC4J4XV62F4
LU WWN Device Id: 5 0014ee 20cd9d7d1
Firmware Version: 82.00A82
User Capacity:    1,000,204,886,016 bytes [1.00 TB]
Sector Sizes:     512 bytes logical, 4096 bytes physical
Rotation Rate:    5400 rpm
Device is:        In smartctl database [for details use: -P show]
ATA Version is:   ACS-2 (minor revision not indicated)
SATA Version is:  SATA 3.0, 6.0 Gb/s (current: 6.0 Gb/s)
Local Time is:    Wed Jul  6 13:21:25 2016 CDT
SMART support is: Available - device has SMART capability.
SMART support is: Enabled
AAM feature is:   Unavailable
APM feature is:   Unavailable
Rd look-ahead is: Enabled
Write cache is:   Enabled
ATA Security is:  Disabled, NOT FROZEN [SEC1]
Wt Cache Reorder: Enabled

=== START OF READ SMART DATA SECTION ===
SMART overall-health self-assessment test result: PASSED

General SMART Values:
Offline data collection status:  (0x00) Offline data collection activity
was never started.
Auto Offline Data Collection: Disabled.
Self-test execution status:      (   0) The previous self-test routine completed
without error or no self-test has ever
been run.
Total time to complete Offline
data collection: (13320) seconds.
Offline data collection
capabilities: (0x7b) SMART execute Offline immediate.
Auto Offline data collection on/off support.
Suspend Offline collection upon new
command.
Offline surface scan supported.
Self-test supported.
Conveyance Self-test supported.
Selective Self-test supported.
SMART capabilities:            (0x0003) Saves SMART data before entering
power-saving mode.
Supports SMART auto save timer.
Error logging capability:        (0x01) Error logging supported.
General Purpose Logging supported.
Short self-test routine
recommended polling time: (   2) minutes.
Extended self-test routine
recommended polling time: ( 152) minutes.
Conveyance self-test routine
recommended polling time: (   5) minutes.
SCT capabilities:       (0x303d) SCT Status supported.
SCT Error Recovery Control supported.
SCT Feature Control supported.
SCT Data Table supported.

SMART Attributes Data Structure revision number: 16
Vendor Specific SMART Attributes with Thresholds:
ID# ATTRIBUTE_NAME          FLAGS    VALUE WORST THRESH FAIL RAW_VALUE
  1 Raw_Read_Error_Rate     POSR-K   200   200   051    -    0
  3 Spin_Up_Time            POS--K   139   139   021    -    4050
  4 Start_Stop_Count        -O--CK   100   100   000    -    23
  5 Reallocated_Sector_Ct   PO--CK   200   200   140    -    0
  7 Seek_Error_Rate         -OSR-K   200   200   000    -    0
  9 Power_On_Hours          -O--CK   100   099   000    -    423
 10 Spin_Retry_Count        -O--CK   100   253   000    -    0
 11 Calibration_Retry_Count -O--CK   100   253   000    -    0
 12 Power_Cycle_Count       -O--CK   100   100   000    -    6
192 Power-Off_Retract_Count -O--CK   200   200   000    -    1
193 Load_Cycle_Count        -O--CK   198   198   000    -    8922
194 Temperature_Celsius     -O---K   115   107   000    -    28
196 Reallocated_Event_Count -O--CK   200   200   000    -    0
197 Current_Pending_Sector  -O--CK   200   200   000    -    0
198 Offline_Uncorrectable   ----CK   100   253   000    -    0
199 UDMA_CRC_Error_Count    -O--CK   200   200   000    -    0
200 Multi_Zone_Error_Rate   ---R--   100   253   000    -    0
                            ||||||_ K auto-keep
                            |||||__ C event count
                            ||||___ R error rate
                            |||____ S speed/performance
                            ||_____ O updated online
                            |______ P prefailure warning

General Purpose Log Directory Version 1
SMART           Log Directory Version 1 [multi-sector log support]
Address    Access  R/W   Size  Description
0x00       GPL,SL  R/O      1  Log Directory
0x01           SL  R/O      1  Summary SMART error log
0x02           SL  R/O      5  Comprehensive SMART error log
0x03       GPL     R/O      6  Ext. Comprehensive SMART error log
0x04       GPL,SL  R/O      8  Device Statistics log
0x06           SL  R/O      1  SMART self-test log
0x07       GPL     R/O      1  Extended self-test log
0x09           SL  R/W      1  Selective self-test log
0x10       GPL     R/O      1  SATA NCQ Queued Error log
0x11       GPL     R/O      1  SATA Phy Event Counters log
0x21       GPL     R/O      1  Write stream error log
0x22       GPL     R/O      1  Read stream error log
0x80-0x9f  GPL,SL  R/W     16  Host vendor specific log
0xa0-0xa7  GPL,SL  VS      16  Device vendor specific log
0xa8-0xb7  GPL,SL  VS       1  Device vendor specific log
0xbd       GPL,SL  VS       1  Device vendor specific log
0xc0       GPL,SL  VS       1  Device vendor specific log
0xc1       GPL     VS      93  Device vendor specific log
0xe0       GPL,SL  R/W      1  SCT Command/Status
0xe1       GPL,SL  R/W      1  SCT Data Transfer

SMART Extended Comprehensive Error Log Version: 1 (6 sectors)
Device Error Count: 1
CR     = Command Register
FEATR  = Features Register
COUNT  = Count (was: Sector Count) Register
LBA_48 = Upper bytes of LBA High/Mid/Low Registers ]  ATA-8
LH     = LBA High (was: Cylinder High) Register    ]   LBA
LM     = LBA Mid (was: Cylinder Low) Register      ] Register
LL     = LBA Low (was: Sector Number) Register     ]
DV     = Device (was: Device/Head) Register
DC     = Device Control Register
ER     = Error register
ST     = Status register
Powered_Up_Time is measured from power on, and printed as
DDd+hh:mm:SS.sss where DD=days, hh=hours, mm=minutes,
SS=sec, and sss=millisec. It "wraps" after 49.710 days.

Error 1 [0] occurred at disk power-on lifetime: 395 hours (16 days + 11 hours)
  When the command that caused the error occurred, the device was
active or idle.

  After command completion occurred, registers were:
  ER -- ST COUNT  LBA_48  LH LM LL DV DC
  -- -- -- == -- == == == -- -- -- -- --
  10 -- 51 00 00 00 00 18 11 28 00 40 00  Error: IDNF at LBA =
0x18112800 = 403777536

  Commands leading to the command that caused the error were:
  CR FEATR COUNT  LBA_48  LH LM LL DV DC  Powered_Up_Time  Command/Feature_Name
  -- == -- == -- == == == -- -- -- -- --  ---------------  --------------------
  61 51 78 00 e0 00 00 18 06 38 00 40 08  5d+03:01:34.882  WRITE FPDMA QUEUED
  61 50 00 00 d8 00 00 18 05 e8 00 40 08  5d+03:01:34.882  WRITE FPDMA QUEUED
  61 50 00 00 d0 00 00 18 05 98 00 40 08  5d+03:01:34.882  WRITE FPDMA QUEUED
  61 50 00 00 c8 00 00 18 05 48 00 40 08  5d+03:01:34.882  WRITE FPDMA QUEUED
  61 50 00 00 c0 00 00 18 04 f8 00 40 08  5d+03:01:34.882  WRITE FPDMA QUEUED

SMART Extended Self-test Log Version: 1 (1 sectors)
No self-tests have been logged.  [To run self-tests, use: smartctl -t]

SMART Selective self-test log data structure revision number 1
 SPAN  MIN_LBA  MAX_LBA  CURRENT_TEST_STATUS
    1        0        0  Not_testing
    2        0        0  Not_testing
    3        0        0  Not_testing
    4        0        0  Not_testing
    5        0        0  Not_testing
Selective self-test flags (0x0):
  After scanning selected spans, do NOT read-scan remainder of disk.
If Selective self-test is pending on power-up, resume after 0 minute delay.

SCT Status Version:                  3
SCT Version (vendor specific):       258 (0x0102)
SCT Support Level:                   1
Device State:                        Active (0)
Current Temperature:                    28 Celsius
Power Cycle Min/Max Temperature:     21/28 Celsius
Lifetime    Min/Max Temperature:     20/36 Celsius
Under/Over Temperature Limit Count:   0/0
Vendor specific:
00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

SCT Temperature History Version:     2
Temperature Sampling Period:         1 minute
Temperature Logging Interval:        1 minute
Min/Max recommended Temperature:      0/60 Celsius
Min/Max Temperature Limit:           -41/85 Celsius
Temperature History Size (Index):    478 (237)

Index    Estimated Time   Temperature Celsius
 238    2016-07-06 05:24    26  *******
 ...    ..( 34 skipped).    ..  *******
 273    2016-07-06 05:59    26  *******
 274    2016-07-06 06:00    27  ********
 ...    ..(  8 skipped).    ..  ********
 283    2016-07-06 06:09    27  ********
 284    2016-07-06 06:10    26  *******
 ...    ..(  3 skipped).    ..  *******
 288    2016-07-06 06:14    26  *******
 289    2016-07-06 06:15    27  ********
 ...    ..( 42 skipped).    ..  ********
 332    2016-07-06 06:58    27  ********
 333    2016-07-06 06:59    28  *********
 ...    ..( 18 skipped).    ..  *********
 352    2016-07-06 07:18    28  *********
 353    2016-07-06 07:19    29  **********
 ...    ..(  3 skipped).    ..  **********
 357    2016-07-06 07:23    29  **********
 358    2016-07-06 07:24    28  *********
 ...    ..( 29 skipped).    ..  *********
 388    2016-07-06 07:54    28  *********
 389    2016-07-06 07:55    29  **********
 390    2016-07-06 07:56    28  *********
 391    2016-07-06 07:57    28  *********
 392    2016-07-06 07:58    29  **********
 393    2016-07-06 07:59    28  *********
 394    2016-07-06 08:00    28  *********
 395    2016-07-06 08:01    29  **********
 ...    ..(  4 skipped).    ..  **********
 400    2016-07-06 08:06    29  **********
 401    2016-07-06 08:07     ?  -
 402    2016-07-06 08:08    21  **
 403    2016-07-06 08:09    21  **
 404    2016-07-06 08:10    21  **
 405    2016-07-06 08:11    22  ***
 406    2016-07-06 08:12    22  ***
 407    2016-07-06 08:13    22  ***
 408    2016-07-06 08:14    24  *****
 409    2016-07-06 08:15    24  *****
 410    2016-07-06 08:16    23  ****
 411    2016-07-06 08:17    23  ****
 412    2016-07-06 08:18    23  ****
 413    2016-07-06 08:19    24  *****
 ...    ..(  2 skipped).    ..  *****
 416    2016-07-06 08:22    24  *****
 417    2016-07-06 08:23    25  ******
 ...    ..(  3 skipped).    ..  ******
 421    2016-07-06 08:27    25  ******
 422    2016-07-06 08:28    26  *******
 ...    ..( 60 skipped).    ..  *******
   5    2016-07-06 09:29    26  *******
   6    2016-07-06 09:30    27  ********
 ...    ..(106 skipped).    ..  ********
 113    2016-07-06 11:17    27  ********
 114    2016-07-06 11:18    26  *******
 ...    ..(113 skipped).    ..  *******
 228    2016-07-06 13:12    26  *******
 229    2016-07-06 13:13    27  ********
 ...    ..(  4 skipped).    ..  ********
 234    2016-07-06 13:18    27  ********
 235    2016-07-06 13:19    26  *******
 236    2016-07-06 13:20    26  *******
 237    2016-07-06 13:21    26  *******

SCT Error Recovery Control:
           Read:     70 (7.0 seconds)
          Write:     70 (7.0 seconds)

Device Statistics (GP Log 0x04)
Page Offset Size         Value  Description
  1  =====  =                =  == General Statistics (rev 2) ==
  1  0x008  4                6  Lifetime Power-On Resets
  1  0x010  4              423  Power-on Hours
  1  0x018  6       2044877667  Logical Sectors Written
  1  0x020  6          2397939  Number of Write Commands
  1  0x028  6       1961443492  Logical Sectors Read
  1  0x030  6          9792433  Number of Read Commands
  3  =====  =                =  == Rotating Media Statistics (rev 1) ==
  3  0x008  4             2800  Spindle Motor Power-on Hours
  3  0x010  4             1582  Head Flying Hours
  3  0x018  4             8924  Head Load Events
  3  0x020  4              200~ Number of Reallocated Logical Sectors
  3  0x028  4                0  Read Recovery Attempts
  3  0x030  4                0  Number of Mechanical Start Failures
  4  =====  =                =  == General Errors Statistics (rev 1) ==
  4  0x008  4                1  Number of Reported Uncorrectable Errors
  4  0x010  4                0  Resets Between Cmd Acceptance and Completion
  5  =====  =                =  == Temperature Statistics (rev 1) ==
  5  0x008  1               28  Current Temperature
  5  0x010  1               27  Average Short Term Temperature
  5  0x018  1               26  Average Long Term Temperature
  5  0x020  1               36  Highest Temperature
  5  0x028  1               20  Lowest Temperature
  5  0x030  1               33  Highest Average Short Term Temperature
  5  0x038  1               22  Lowest Average Short Term Temperature
  5  0x040  1               27  Highest Average Long Term Temperature
  5  0x048  1               25  Lowest Average Long Term Temperature
  5  0x050  4                0  Time in Over-Temperature
  5  0x058  1               60  Specified Maximum Operating Temperature
  5  0x060  4                0  Time in Under-Temperature
  5  0x068  1                0  Specified Minimum Operating Temperature
  6  =====  =                =  == Transport Statistics (rev 1) ==
  6  0x008  4               96  Number of Hardware Resets
  6  0x010  4               45  Number of ASR Events
  6  0x018  4                0  Number of Interface CRC Errors
                              |_ ~ normalized value

SATA Phy Event Counters (GP Log 0x11)
ID      Size     Value  Description
0x0001  2            0  Command failed due to ICRC error
0x0002  2            0  R_ERR response for data FIS
0x0003  2            0  R_ERR response for device-to-host data FIS
0x0004  2            0  R_ERR response for host-to-device data FIS
0x0005  2            0  R_ERR response for non-data FIS
0x0006  2            0  R_ERR response for device-to-host non-data FIS
0x0007  2            0  R_ERR response for host-to-device non-data FIS
0x0008  2            0  Device-to-host non-data FIS retries
0x0009  2            8  Transition from drive PhyRdy to drive PhyNRdy
0x000a  2           14  Device-to-host register FISes sent due to a COMRESET
0x000b  2            0  CRC errors within host-to-device FIS
0x000f  2            0  R_ERR response for host-to-device data FIS, CRC
0x0012  2            0  R_ERR response for host-to-device non-data FIS, CRC
0x8000  4        24888  Vendor specific

^ permalink raw reply

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Ming Lei @ 2016-07-06 15:57 UTC (permalink / raw)
  To: Lars Ellenberg
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <20160706123841.GA13335@soda.linbit>

On Wed, Jul 6, 2016 at 8:38 PM, Lars Ellenberg
<lars.ellenberg@linbit.com> wrote:
> On Mon, Jul 04, 2016 at 06:47:29PM +0800, Ming Lei wrote:
>> >> One clean solution may be to convert the loop of generic_make_request()
>> >> into the following way:
>> >>
>> >> do {
>> >>     struct bio *splitted, *remainder = NULL;
>> >>     struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>> >>
>> >>     blk_queue_split(q, &bio, &remainder, q->bio_split);
>> >>
>> >>     ret = q->make_request_fn(q, bio);
>> >>
>> >>     if (remainder)
>> >>          bio_list_add(current->bio_list, remainder);
>> >>     bio = bio_list_pop(current->bio_list);
>> >> } while (bio)
>> >
>> > Not good enough.
>
>
>> > Goal was to first process all "deeper level" bios
>> > before processing the remainder.
>>
>> For the reported bio splitting issue, I think the goal is to make sure all
>> BIOs generated from 'bio' inside .make_request_fn(bio) are queued
>> before the 'current' remainder. Cause it is the issue introduced by
>> blk_split_bio().

I believe the above description is correct, but my previous solution
is wrong, looks what we need is a binary tree, in which left child is
the splitted bio and the remainder bio is the right child, and the tree
need to be traversed in pre-order to make sure all BIOs generated
from 'bio' inside .make_request_fn(bio) are queued before the
remainder of bio splitting.

Another property of the tree is that if one node has only one child,
the child has to be left one.

I will think about it further to see if it is doable to implement.

>
> Stacking:
>         qA (limitting layer)
>         -> qB (remapping)
>         -> qC (remapping)
>         -> qD (hardware)
>
> [in fact you don't even need four layers,
>  this is just to clarify that the stack may be more complex than you
>  assume it would be]
>
> Columns below:
> 1) argument to generic_make_request() and its target queue.
> 2) current->bio_list
> 3) "in-flight" counter of qA.
>
> ==== In your new picture, iiuc:
>
> generic_make_request(bio_orig)
>                 NULL                    in-flight=0
> bio_orig        empty
>   blk_queue_split()
>   result:
>   bio_s, bio_r
> qA->make_request_fn(bio_s)
>                                         in-flight=1
>   bio_c = bio_clone(bio_s)
>   generic_make_request(bio_c to qB)
>                 bio_c
> <-return
>   bio_list_add(bio_r)
>                 bio_c, bio_r
>   bio_list_pop()
>                 bio_r
> qB->make_request_fn(bio_c)
>   (Assume it does not clone, but only remap.
>   But it may also be a striping layer,
>   and queue more than one bio here.)
>   generic_make_request(bio_c to qC)
>                 bio_r, bio_c
> <-return
>   bio_list_pop()
>                 bio_c
> qA->make_request_fn(bio_r)              in-flight still 1
>
>         BLOCKS, because bio_c has not been processed to its final
>         destination qD yet, and not dispatched to hardware.

Yes, you are right.

>
>
> ==== my suggestion
>
> generic_make_request(bio_orig)
>                 NULL                    in-flight=0
> bio_orig        empty                   in-flight=0
> qA->make_request_fn(bio_orig)
>   blk_queue_split()
>   result:
>   bio_s, and bio_r stuffed away to head of remainder list.
>                                         in-flight=1
>   bio_c = bio_clone(bio_s)
>   generic_make_request(bio_c to qB)
>                 bio_c
> <-return
>                 bio_c
>   bio_list_pop()
>                 empty
> qB->make_request_fn(bio_c)
>   (Assume it does not clone, but only remap.
>   But it may also be a striping layer,
>   and queue more than one bio here.)
>   generic_make_request(bio_c to qC)
>                 bio_c
> <-return
>   bio_list_pop()
>                 empty
> qC->make_request_fn(bio_c)
>   generic_make_request(bio_c to qD)
>                 bio_c
> <-return
>   bio_list_pop()
>                 empty
> qD->make_request_fn(bio_c)
>         dispatches to hardware
> <-return
>                 empty
>    bio_list_pop()
>    NULL, great, lets pop from remainder list
> qA->make_request_fn(bio_r)              in-flight=?
>
>         May block, but only until completion of bio_c.
>         Which may already have happened.
>
>         *makes progress*

I admit your solution is smart, but it isn't easy to prove it as correct
in theory.  But if the traversal can be mapped into pre-order traversal
of the above binary tree, it may be correct.

Thanks,
Ming

>
> Thanks,
>
>     Lars
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" 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

* Re: [PATCH RESENT] dm: Check kthread_run's return value
From: Minfei Huang @ 2016-07-06 13:38 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: agk, shli, dm-devel, linux-raid, linux-kernel
In-Reply-To: <20160706133129.GB28379@redhat.com>

On 07/06/16 at 09:31P, Mike Snitzer wrote:
> On Wed, Jul 06 2016 at  9:27am -0400,
> Minfei Huang <mnghuan@gmail.com> wrote:
> 
> > On 07/06/16 at 09:16P, Mike Snitzer wrote:
> > > On Mon, Jul 04 2016 at 11:25am -0400,
> > > Minfei Huang <mnghuan@gmail.com> wrote:
> > > 
> > > > kthread function is used to process kthread_work. And there is no return
> > > > value checking during create this thread. Add this checking to fix this
> > > > issue.
> > > > 
> > > > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> > > This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> > > this fix for 4.8 inclusion, see:
> > > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad
> > 
> > Seems that we should fix it in stable as well.
> 
> Given the code movement it isn't easy to do (by simply adding a stable@
> cc).  I've not seen a single report of an ignored kthread_run() failure
> for multipath using .request_fn interface.

Yep, this bug will be trigged in very restrict condition, also dm is
installed in boot time when there are a lot of memory avaiable.

Thanks
Minfei

^ permalink raw reply

* Re: [PATCH RESENT] dm: Check kthread_run's return value
From: Mike Snitzer @ 2016-07-06 13:31 UTC (permalink / raw)
  To: Minfei Huang; +Cc: agk, shli, dm-devel, linux-raid, linux-kernel
In-Reply-To: <20160706132755.GA1712@MinfeideMacBook-Pro.local>

On Wed, Jul 06 2016 at  9:27am -0400,
Minfei Huang <mnghuan@gmail.com> wrote:

> On 07/06/16 at 09:16P, Mike Snitzer wrote:
> > On Mon, Jul 04 2016 at 11:25am -0400,
> > Minfei Huang <mnghuan@gmail.com> wrote:
> > 
> > > kthread function is used to process kthread_work. And there is no return
> > > value checking during create this thread. Add this checking to fix this
> > > issue.
> > > 
> > > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> > This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> > this fix for 4.8 inclusion, see:
> > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad
> 
> Seems that we should fix it in stable as well.

Given the code movement it isn't easy to do (by simply adding a stable@
cc).  I've not seen a single report of an ignored kthread_run() failure
for multipath using .request_fn interface.

^ permalink raw reply

* Re: [PATCH RESENT] dm: Check kthread_run's return value
From: Minfei Huang @ 2016-07-06 13:27 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: agk, shli, dm-devel, linux-raid, linux-kernel
In-Reply-To: <20160706131601.GA28379@redhat.com>

On 07/06/16 at 09:16P, Mike Snitzer wrote:
> On Mon, Jul 04 2016 at 11:25am -0400,
> Minfei Huang <mnghuan@gmail.com> wrote:
> 
> > kthread function is used to process kthread_work. And there is no return
> > value checking during create this thread. Add this checking to fix this
> > issue.
> > 
> > Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
> this fix for 4.8 inclusion, see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad

Seems that we should fix it in stable as well.

Thanks
Minfei

^ permalink raw reply

* Re: [PATCH RESENT] dm: Check kthread_run's return value
From: Mike Snitzer @ 2016-07-06 13:16 UTC (permalink / raw)
  To: Minfei Huang; +Cc: agk, shli, dm-devel, linux-raid, linux-kernel
In-Reply-To: <1467645927-4143-1-git-send-email-mnghuan@gmail.com>

On Mon, Jul 04 2016 at 11:25am -0400,
Minfei Huang <mnghuan@gmail.com> wrote:

> kthread function is used to process kthread_work. And there is no return
> value checking during create this thread. Add this checking to fix this
> issue.
> 
> Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> ---
>  drivers/md/dm.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 1b2f962..d68b9d2 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -2654,12 +2654,15 @@ struct queue_limits *dm_get_queue_limits(struct mapped_device *md)
>  }
>  EXPORT_SYMBOL_GPL(dm_get_queue_limits);
>  
> -static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
> +static int dm_old_init_rq_based_worker_thread(struct mapped_device *md)
>  {
>  	/* Initialize the request-based DM worker thread */
>  	init_kthread_worker(&md->kworker);
>  	md->kworker_task = kthread_run(kthread_worker_fn, &md->kworker,
>  				       "kdmwork-%s", dm_device_name(md));
> +	if (IS_ERR(md->kworker_task))
> +		return -ENOMEM;
> +	return 0;
>  }
>  
>  /*
> @@ -2667,6 +2670,8 @@ static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
>   */
>  static int dm_old_init_request_queue(struct mapped_device *md)
>  {
> +	int ret;
> +
>  	/* Fully initialize the queue */
>  	if (!blk_init_allocated_queue(md->queue, dm_request_fn, NULL))
>  		return -EINVAL;
> @@ -2678,7 +2683,9 @@ static int dm_old_init_request_queue(struct mapped_device *md)
>  	blk_queue_softirq_done(md->queue, dm_softirq_done);
>  	blk_queue_prep_rq(md->queue, dm_old_prep_fn);
>  
> -	dm_old_init_rq_based_worker_thread(md);
> +	ret = dm_old_init_rq_based_worker_thread(md);
> +	if (ret < 0)
> +		return ret;
>  
>  	elv_register_queue(md->queue);
>  

This needed rebasing against linux-dm.git's 'for-next'.  I've now staged
this fix for 4.8 inclusion, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=7193a9defcab6f3d3f1eb64c68bad7534e5a39ad

^ permalink raw reply

* Re: Request for assistance
From: Wols Lists @ 2016-07-06 12:51 UTC (permalink / raw)
  To: o1bigtenor, Adam Goryachev; +Cc: Linux-RAID
In-Reply-To: <CAPpdf59gF6BMpmCY5SNzNH2O_WFn17hMVnrjHAyA0pU_i+--uA@mail.gmail.com>

On 06/07/16 13:14, o1bigtenor wrote:
> On Tue, Jul 5, 2016 at 8:55 PM, Adam Goryachev
> <mailinglists@websitemanagers.com.au> wrote:
>> On 06/07/16 10:13, o1bigtenor wrote:
>>>
>>> Greetings
>>>
>>> Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
>>> gets significant lightning and also brownout (rural power) events.
>>>
> snip
>>>
>>> Do I just re-create the array?
>>>
>> No, not if you value your data. Only re-create the array if you are told to
>> by someone (knowledgeable) on the list.
>>
>> In your case, I think you should stop the array.
>> mdadm --stop /dev/md0
>> Make sure there is nothing listed in /proc/mdstat
>> Then try to assemble the array, but force the events to match:
>> mdadm --assemble /dev/md0 --force /dev/sd[bcef]1
>>
>> If that doesn't work, then include the output from dmesg as well as
>> /proc/mdstat and any commandline output generated.
>>
>> You might also want to examine why two drives dropped, referring to logs or
>> similar might assist.
>>
> mdadm --stop /dev/md0
> cat /proc/mdstat
>     indicated no md (can't remember the exact response but it said
> nothing there)
> mdadm --assemble /dev/md0 --force /dev/sd[bcef]1 to
> 
> mdadm :forcing event count in /dev/sde1(2) from 64841 to 64844
> mdadm :forcing event count in /dev/sdf1(3) from 64841 to 64844
> mdadm: clearing FAULTY flag for device 3 in /dev/md0 for /dev/sdf1
> mdadm: Marking array /dev/md0 as 'clean'
> mdadm: /dev/md0 has been started with 4 drives
> 
> So my array is back up - - - thank you very much for your assistance!!!
> 
But why did they drop ... are you using desktop drives? I use Seagate
Barracudas - NOT a particularly good idea. You should be using WD Red,
Seagate NAS, or similar.

"smartctl -x /dev/sdx" will give you an idea of what's going on. Search
the list for "timeout error" for an idea of the grief you'll get if
you're using desktop drives ...

If smartctl says smart is disabled, enable it. When I do, my drive comes
back (using the -x option again) saying "SCT Error Recovery not
supported". This is a no-no for a decent raid drive. I think the other
acronyms are ETL or TLS - either way you can control how the drive
reports an error back to the OS. Which is why you need proper raid
drives (the manufacturers have downgraded the firmware on desktop drives :-(

You need to fix the WHY or it could easily happen again. And this could
well be why ... (if you've had a problem on a desktop drive, it WILL
happen again, and data loss is quite likely ... even if you recover the
bulk of the drive).

Cheers,
Wol


^ permalink raw reply

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-06 12:38 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <CACVXFVNTKsRQ0PQTL47-ceP-KqMR-zR4DO1v-ey5NTUYv8bUog@mail.gmail.com>

On Mon, Jul 04, 2016 at 06:47:29PM +0800, Ming Lei wrote:
> >> One clean solution may be to convert the loop of generic_make_request()
> >> into the following way:
> >>
> >> do {
> >>     struct bio *splitted, *remainder = NULL;
> >>     struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> >>
> >>     blk_queue_split(q, &bio, &remainder, q->bio_split);
> >>
> >>     ret = q->make_request_fn(q, bio);
> >>
> >>     if (remainder)
> >>          bio_list_add(current->bio_list, remainder);
> >>     bio = bio_list_pop(current->bio_list);
> >> } while (bio)
> >
> > Not good enough.


> > Goal was to first process all "deeper level" bios
> > before processing the remainder.
> 
> For the reported bio splitting issue, I think the goal is to make sure all
> BIOs generated from 'bio' inside .make_request_fn(bio) are queued
> before the 'current' remainder. Cause it is the issue introduced by
> blk_split_bio().

Stacking:
	qA (limitting layer)
	-> qB (remapping)
	-> qC (remapping)
	-> qD (hardware)

[in fact you don't even need four layers,
 this is just to clarify that the stack may be more complex than you
 assume it would be]

Columns below:
1) argument to generic_make_request() and its target queue.
2) current->bio_list
3) "in-flight" counter of qA.

==== In your new picture, iiuc:

generic_make_request(bio_orig)
		NULL			in-flight=0
bio_orig	empty
  blk_queue_split()
  result:
  bio_s, bio_r	
qA->make_request_fn(bio_s)
					in-flight=1
  bio_c = bio_clone(bio_s)
  generic_make_request(bio_c to qB)
		bio_c
<-return
  bio_list_add(bio_r)
  		bio_c, bio_r
  bio_list_pop()
  		bio_r
qB->make_request_fn(bio_c)
  (Assume it does not clone, but only remap.
  But it may also be a striping layer,
  and queue more than one bio here.)
  generic_make_request(bio_c to qC)
  		bio_r, bio_c
<-return
  bio_list_pop()
  		bio_c
qA->make_request_fn(bio_r)		in-flight still 1

	BLOCKS, because bio_c has not been processed to its final
	destination qD yet, and not dispatched to hardware.


==== my suggestion

generic_make_request(bio_orig)
		NULL			in-flight=0
bio_orig	empty			in-flight=0
qA->make_request_fn(bio_orig)
  blk_queue_split()
  result:
  bio_s, and bio_r stuffed away to head of remainder list.
					in-flight=1
  bio_c = bio_clone(bio_s)
  generic_make_request(bio_c to qB)
		bio_c
<-return
  		bio_c
  bio_list_pop()
  		empty
qB->make_request_fn(bio_c)
  (Assume it does not clone, but only remap.
  But it may also be a striping layer,
  and queue more than one bio here.)
  generic_make_request(bio_c to qC)
  		bio_c
<-return
  bio_list_pop()
  		empty
qC->make_request_fn(bio_c)
  generic_make_request(bio_c to qD)
  		bio_c
<-return
  bio_list_pop()
  		empty
qD->make_request_fn(bio_c)
	dispatches to hardware
<-return
		empty
   bio_list_pop()
   NULL, great, lets pop from remainder list
qA->make_request_fn(bio_r)		in-flight=?

	May block, but only until completion of bio_c.
	Which may already have happened.

	*makes progress*

Thanks,

    Lars

^ permalink raw reply

* Re: Request for assistance
From: o1bigtenor @ 2016-07-06 12:15 UTC (permalink / raw)
  To: keld; +Cc: Linux-RAID
In-Reply-To: <20160706073934.GA8404@www5.open-std.org>

On Wed, Jul 6, 2016 at 2:39 AM,  <keld@keldix.com> wrote:
> What operating system and version are you  running?
>

Running Debian testing.

Thanks for the assistance.

Dee

^ permalink raw reply

* Re: Request for assistance
From: o1bigtenor @ 2016-07-06 12:14 UTC (permalink / raw)
  To: Adam Goryachev; +Cc: Linux-RAID
In-Reply-To: <577C64FA.6050504@websitemanagers.com.au>

On Tue, Jul 5, 2016 at 8:55 PM, Adam Goryachev
<mailinglists@websitemanagers.com.au> wrote:
> On 06/07/16 10:13, o1bigtenor wrote:
>>
>> Greetings
>>
>> Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
>> gets significant lightning and also brownout (rural power) events.
>>
snip
>>
>> Do I just re-create the array?
>>
> No, not if you value your data. Only re-create the array if you are told to
> by someone (knowledgeable) on the list.
>
> In your case, I think you should stop the array.
> mdadm --stop /dev/md0
> Make sure there is nothing listed in /proc/mdstat
> Then try to assemble the array, but force the events to match:
> mdadm --assemble /dev/md0 --force /dev/sd[bcef]1
>
> If that doesn't work, then include the output from dmesg as well as
> /proc/mdstat and any commandline output generated.
>
> You might also want to examine why two drives dropped, referring to logs or
> similar might assist.
>
mdadm --stop /dev/md0
cat /proc/mdstat
    indicated no md (can't remember the exact response but it said
nothing there)
mdadm --assemble /dev/md0 --force /dev/sd[bcef]1 to

mdadm :forcing event count in /dev/sde1(2) from 64841 to 64844
mdadm :forcing event count in /dev/sdf1(3) from 64841 to 64844
mdadm: clearing FAULTY flag for device 3 in /dev/md0 for /dev/sdf1
mdadm: Marking array /dev/md0 as 'clean'
mdadm: /dev/md0 has been started with 4 drives

So my array is back up - - - thank you very much for your assistance!!!

What does the 'clearing FAULTY flag . . ' mean?

Regards

Dee

^ permalink raw reply

* Re: Request for assistance
From: keld @ 2016-07-06  7:39 UTC (permalink / raw)
  To: o1bigtenor; +Cc: Linux-RAID
In-Reply-To: <CAPpdf58z4JagWmP=xiUj9qOU-bdyN7W=EvcM9evjCT4ZWp+VxQ@mail.gmail.com>

What operating system and version are you  running?

Best regards
keld

On Tue, Jul 05, 2016 at 07:13:23PM -0500, o1bigtenor wrote:
> Greetings
> 
> Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
> gets significant lightning and also brownout (rural power) events.
> 
> Found the array was read only this morning. Thought that rebooting the
> system might correct things - - - it did not as the array did not
> load.
> 
> commands used followed by system response
> 
> mdadm --detail /dev/md0
>    mdadm:  md device /dev/md0 does not appear to be active.
> 
> cat /proc/mdstat
>    md0  : inactive sdc1[5](S) sdf1[8](S) sde1[7](S) sdb1[4](S)
> 
> mdadm -E /dev/sdb1
>                        sdc1
>                        sde1
>                        sdf1
> 
> everything is the same except for 2 items
> 
> sde and sdf have uptime listed from July 04 05:50:46
>                           events 64841
>                           array state of AAAA
> 
> sdb and sdc have uptime listed from July 05 01:57:38
>                            events 64844
>                            array state of AAA.
> 
> 
> 
> Do I just re-create the array?
> 
> TIA
> 
> Dee
> --
> 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

* Re: Request for assistance
From: Adam Goryachev @ 2016-07-06  1:55 UTC (permalink / raw)
  To: o1bigtenor, Linux-RAID
In-Reply-To: <CAPpdf58z4JagWmP=xiUj9qOU-bdyN7W=EvcM9evjCT4ZWp+VxQ@mail.gmail.com>

On 06/07/16 10:13, o1bigtenor wrote:
> Greetings
>
> Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
> gets significant lightning and also brownout (rural power) events.
>
> Found the array was read only this morning. Thought that rebooting the
> system might correct things - - - it did not as the array did not
> load.
>
> commands used followed by system response
>
> mdadm --detail /dev/md0
>     mdadm:  md device /dev/md0 does not appear to be active.
>
> cat /proc/mdstat
>     md0  : inactive sdc1[5](S) sdf1[8](S) sde1[7](S) sdb1[4](S)
>
> mdadm -E /dev/sdb1
>                         sdc1
>                         sde1
>                         sdf1
>
> everything is the same except for 2 items
>
> sde and sdf have uptime listed from July 04 05:50:46
>                            events 64841
>                            array state of AAAA
>
> sdb and sdc have uptime listed from July 05 01:57:38
>                             events 64844
>                             array state of AAA.
>
>
>
> Do I just re-create the array?
>
No, not if you value your data. Only re-create the array if you are told 
to by someone (knowledgeable) on the list.

In your case, I think you should stop the array.
mdadm --stop /dev/md0
Make sure there is nothing listed in /proc/mdstat
Then try to assemble the array, but force the events to match:
mdadm --assemble /dev/md0 --force /dev/sd[bcef]1

If that doesn't work, then include the output from dmesg as well as 
/proc/mdstat and any commandline output generated.

You might also want to examine why two drives dropped, referring to logs 
or similar might assist.

Regards,
Adam

-- 
Adam Goryachev Website Managers www.websitemanagers.com.au

^ permalink raw reply

* Request for assistance
From: o1bigtenor @ 2016-07-06  0:13 UTC (permalink / raw)
  To: Linux-RAID

Greetings

Running a Raid 10 array with 4 - 3 TB drives. Have a UPS but this area
gets significant lightning and also brownout (rural power) events.

Found the array was read only this morning. Thought that rebooting the
system might correct things - - - it did not as the array did not
load.

commands used followed by system response

mdadm --detail /dev/md0
   mdadm:  md device /dev/md0 does not appear to be active.

cat /proc/mdstat
   md0  : inactive sdc1[5](S) sdf1[8](S) sde1[7](S) sdb1[4](S)

mdadm -E /dev/sdb1
                       sdc1
                       sde1
                       sdf1

everything is the same except for 2 items

sde and sdf have uptime listed from July 04 05:50:46
                          events 64841
                          array state of AAAA

sdb and sdc have uptime listed from July 05 01:57:38
                           events 64844
                           array state of AAA.



Do I just re-create the array?

TIA

Dee

^ permalink raw reply

* Re: [PATCH v3 1/3] bcache: Remove redundant parameter for cache_alloc()
From: Jens Axboe @ 2016-07-05 17:35 UTC (permalink / raw)
  To: Yijing Wang, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel
In-Reply-To: <1467595405-20805-1-git-send-email-wangyijing@huawei.com>

On 07/03/2016 07:23 PM, Yijing Wang wrote:
> Cache_sb is not used in cache_alloc, and we have copied
> sb info to cache->sb already, remove it.

Added this, and 2-3/3 for 4.8, thanks.

-- 
Jens Axboe

^ permalink raw reply

* [PATCH] Monitor: release /proc/mdstat fd when no arrays present
From: Tomasz Majchrzak @ 2016-07-05  7:12 UTC (permalink / raw)
  To: linux-raid
  Cc: Jes.Sorensen, aleksey.obitotskiy, pawel.baldysiak,
	artur.paszkiewicz

If md kernel module is reloaded, /proc/mdstat cannot be accessed ("cat:
/proc/mdstat: No such file or directory"). The reason is mdadm monitor
still holds a file descriptor to previous /proc/mdstat instance. It
leads to really confusing outcome of the following operations - mdadm
seems to run without errors, however some udev rules don't get executed
and new array doesn't work.

Add a check if lseek was successful as it fails if md kernel module has
been unloaded - close a file descriptor then. The problem is mdadm
monitor doesn't always do it before next operation takes place. To
prevent it monitor always releases /proc/mdstat descriptor when there
are no arrays to be monitored, just in case driver unload happens in a
moment.

Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
 Monitor.c | 2 ++
 mdstat.c  | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Monitor.c b/Monitor.c
index 4adc237..802a9d9 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -213,6 +213,8 @@ int Monitor(struct mddev_dev *devlist,
 		if (mdstat)
 			free_mdstat(mdstat);
 		mdstat = mdstat_read(oneshot?0:1, 0);
+		if (!mdstat)
+			mdstat_close();
 
 		for (st=statelist; st; st=st->next)
 			if (check_array(st, mdstat, c->test, &info,
diff --git a/mdstat.c b/mdstat.c
index 2972cdf..3962896 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -133,7 +133,11 @@ struct mdstat_ent *mdstat_read(int hold, int start)
 	int fd;
 
 	if (hold && mdstat_fd != -1) {
-		lseek(mdstat_fd, 0L, 0);
+		off_t offset = lseek(mdstat_fd, 0L, 0);
+		if (offset == (off_t)-1) {
+			mdstat_close();
+			return NULL;
+		}
 		fd = dup(mdstat_fd);
 		if (fd >= 0)
 			f = fdopen(fd, "r");
-- 
1.8.3.1


^ permalink raw reply related

* Re: URE, link resets, user hostile defaults
From: Pasi Kärkkäinen @ 2016-07-04 21:43 UTC (permalink / raw)
  To: Zygo Blaxell; +Cc: Chris Murphy, Hannes Reinecke, linux-raid
In-Reply-To: <20160629121751.GT15597@hungrycats.org>

On Wed, Jun 29, 2016 at 08:17:51AM -0400, Zygo Blaxell wrote:
> On Tue, Jun 28, 2016 at 11:33:36AM -0600, Chris Murphy wrote:
> > On Tue, Jun 28, 2016 at 12:33 AM, Hannes Reinecke <hare@suse.de> wrote:
> > > Can you post a message log detailing this problem?
> >
> > Just over the weekend Phil Turmel posted an email with a bunch of back
> > reading on the subject of timeout mismatches for someone to read. I've
> > lost track of how many user emails he's replied to, discovering this
> > common misconfiguration, and get it straightened out and more often
> > than not helping the user recover data that otherwise would have been
> > lost *because* of hard link resetting instead of explicit read errors.
> 
> OK, but the two links you provided are not examples of these.
> 

Here's one of the threads where Phil explains the issue:

http://marc.info/?l=linux-raid&m=133665797115876&w=2

quote:


"A very common report I see on this mailing list is people who have lost arrays 
where the drives all appear to be healthy.  
Given the large size of today's hard drives, even healthy drives will occasionally 
have an unrecoverable read error.

When this happens in a raid array with a desktop drive without SCTERC,
the driver times out and reports an error to MD.  MD proceeds to
reconstruct the missing data and tries to write it back to the bad
sector.  However, that drive is still trying to read the bad sector and
ignores the controller.  The write is immediately rejected.  BOOM!  The
*write* error ejects that member from the array.  And you are now
degraded.

If you don't notice the degraded array right away, you probably won't
notice until a URE on another drive pops up.  Once that happens, you
can't complete a resync to revive the array.

Running a "check" or "repair" on an array without TLER will have the
opposite of the intended effect: any URE will kick a drive out instead
of fixing it.

In the same scenario with an enterprise drive, or a drive with SCTERC
turned on, the drive read times out before the controller driver, the
controller never resets the link to the drive, and the followup write
succeeds.  (The sector is either successfully corrected in place, or
it is relocated by the drive.)  No BOOM."



-- Pasi


^ permalink raw reply

* [PATCH RESENT] dm: Check kthread_run's return value
From: Minfei Huang @ 2016-07-04 15:25 UTC (permalink / raw)
  To: agk, snitzer, shli; +Cc: dm-devel, linux-raid, linux-kernel, Minfei Huang

kthread function is used to process kthread_work. And there is no return
value checking during create this thread. Add this checking to fix this
issue.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
---
 drivers/md/dm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1b2f962..d68b9d2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2654,12 +2654,15 @@ struct queue_limits *dm_get_queue_limits(struct mapped_device *md)
 }
 EXPORT_SYMBOL_GPL(dm_get_queue_limits);
 
-static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
+static int dm_old_init_rq_based_worker_thread(struct mapped_device *md)
 {
 	/* Initialize the request-based DM worker thread */
 	init_kthread_worker(&md->kworker);
 	md->kworker_task = kthread_run(kthread_worker_fn, &md->kworker,
 				       "kdmwork-%s", dm_device_name(md));
+	if (IS_ERR(md->kworker_task))
+		return -ENOMEM;
+	return 0;
 }
 
 /*
@@ -2667,6 +2670,8 @@ static void dm_old_init_rq_based_worker_thread(struct mapped_device *md)
  */
 static int dm_old_init_request_queue(struct mapped_device *md)
 {
+	int ret;
+
 	/* Fully initialize the queue */
 	if (!blk_init_allocated_queue(md->queue, dm_request_fn, NULL))
 		return -EINVAL;
@@ -2678,7 +2683,9 @@ static int dm_old_init_request_queue(struct mapped_device *md)
 	blk_queue_softirq_done(md->queue, dm_softirq_done);
 	blk_queue_prep_rq(md->queue, dm_old_prep_fn);
 
-	dm_old_init_rq_based_worker_thread(md);
+	ret = dm_old_init_rq_based_worker_thread(md);
+	if (ret < 0)
+		return ret;
 
 	elv_register_queue(md->queue);
 
-- 
2.6.3

^ permalink raw reply related

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Ming Lei @ 2016-07-04 10:47 UTC (permalink / raw)
  To: Lars Ellenberg
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <20160704082006.GN3239@soda.linbit>

On Mon, Jul 4, 2016 at 4:20 PM, Lars Ellenberg
<lars.ellenberg@linbit.com> wrote:
> On Sat, Jul 02, 2016 at 06:28:29PM +0800, Ming Lei wrote:
>> The idea looks good, but not sure it can cover all cases of
>> dm over brbd or brbd over dm and maintaining two lists
>> becomes too complicated.
>>
>> One clean solution may be to convert the loop of generic_make_request()
>> into the following way:
>>
>> do {
>>     struct bio *splitted, *remainder = NULL;
>>     struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>>
>>     blk_queue_split(q, &bio, &remainder, q->bio_split);
>>
>>     ret = q->make_request_fn(q, bio);
>>
>>     if (remainder)
>>          bio_list_add(current->bio_list, remainder);
>>     bio = bio_list_pop(current->bio_list);
>> } while (bio)
>
> Not good enough.
>
> Consider DRBD on device-mapper on device-mapper on scsi,
> or insert multipath and / or md raid into the stack.
> The iterative call to q->make_request_fn() in the second iteration
> may queue bios after the remainder.

But this remainder is not the top remainder any more, I guess you mean
the following situation:

- drbd_make_request(bio)
       ->generic_make_request(bio)
              ->bio is added into current->bio_list
- bio is splitted as bio_a and bio_b in generic_make_request()
- dm_make_request(bio_a)
        ->generic_make_request(bio_a)
               ->bio_a is add into current_list
- bio_a is splitted as bio_a_a and bio_a_b in generic_make_request()
- dm_make_request(bio_a_a)
      ->.....
- bio_a_b is added into current->bio_list
- dm_make_request(bio_a_b)

But it is correct because bio_a depends on both bio_a_a and bio_a_b.
Or I understand you wrong?

>
> Goal was to first process all "deeper level" bios
> before processing the remainder.

For the reported bio splitting issue, I think the goal is to make sure all
BIOs generated from 'bio' inside .make_request_fn(bio) are queued
before the 'current' remainder. Cause it is the issue introduced by
blk_split_bio().

Thanks,
Ming

>
> You can achieve this by doing last-in-first-out on bio_list,
> or by using two lists, as I suggested in the original post.
>
>     Lars
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" 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

* Re: [RFC] block: fix blk_queue_split() resource exhaustion
From: Lars Ellenberg @ 2016-07-04  8:20 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-block, Roland Kammerer, Jens Axboe, NeilBrown,
	Kent Overstreet, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	open list:DEVICE-MAPPER (LVM), Ingo Molnar, Peter Zijlstra,
	Takashi Iwai, Jiri Kosina, Zheng Liu, Keith Busch,
	Martin K. Petersen, Kirill A. Shutemov, Linux Kernel Mailing List,
	open list:BCACHE (BLOCK LAYER CACHE),
	open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <CACVXFVMTp17gcM8HEMrZ+0TJWgpGk+VWDgC7t+rMAancWZEFFw@mail.gmail.com>

On Sat, Jul 02, 2016 at 06:28:29PM +0800, Ming Lei wrote:
> The idea looks good, but not sure it can cover all cases of
> dm over brbd or brbd over dm and maintaining two lists
> becomes too complicated.
> 
> One clean solution may be to convert the loop of generic_make_request()
> into the following way:
> 
> do {
>     struct bio *splitted, *remainder = NULL;
>     struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> 
>     blk_queue_split(q, &bio, &remainder, q->bio_split);
> 
>     ret = q->make_request_fn(q, bio);
> 
>     if (remainder)
>          bio_list_add(current->bio_list, remainder);
>     bio = bio_list_pop(current->bio_list);
> } while (bio)

Not good enough.

Consider DRBD on device-mapper on device-mapper on scsi,
or insert multipath and / or md raid into the stack.
The iterative call to q->make_request_fn() in the second iteration
may queue bios after the remainder.

Goal was to first process all "deeper level" bios
before processing the remainder.

You can achieve this by doing last-in-first-out on bio_list,
or by using two lists, as I suggested in the original post.

    Lars

^ permalink raw reply

* raid5/6: general protection fault in async_copy_data
From: Joey Liao @ 2016-07-04  8:07 UTC (permalink / raw)
  To: linux-raid

Hi,

We meet a general protection fault issue when doing the I/O stress
test. (Please refer the following call trace.)

The linux version what we use is 3.19.8.

Besides, it will not only happen in raid5 but also raid6, and our raid
is normal and clean (not in resync or rebuild or degraded state) in
this case.
The general protection fault logs just show-up suddenly during the I/O
stress test, and no other logs else are before it.

It seems that it is an old issue that someone has been meet in linux 3.8.13.
http://comments.gmane.org/gmane.linux.raid/48737

However, it did not come to an exact conclusion then.
Does anyone have any idea about this situation?


<4>[ 8415.258965] general protection fault: 0000 [#1] SMP
<4>[ 8415.263946] Modules linked in: vfio_pci vfio_iommu_type1 vfio
vringh virtio_scsi virtio_pci virtio_mmio virtio_console
virtio_balloon virtio_rng virtio_blk virtio_net virtio_ring virtio
vhost_net vhost tun macvtap macvlan kvm_intel kvm fbdisk(O) xt_mark
ipt_MASQUERADE iptable_nat nf_nat_masquerade_ipv4 nf_nat_ipv4 nf_nat
ppp_deflate bsd_comp ppp_mppe ppp_async ppp_generic slhc iscsi_tcp(O)
libiscsi_tcp(O) libiscsi(O) scsi_transport_iscsi(O) btusb bluetooth
bonding bridge stp ipv6 uvcvideo videobuf2_vmalloc videobuf2_memops
videobuf2_core snd_usb_caiaq snd_usb_audio snd_usbmidi_lib
snd_seq_midi snd_rawmidi fnotify(PO) udf isofs iTCO_wdt psnap llc
ufsd(PO) jnl(O) pl2303 usbserial intel_ips drbd(O) flashcache(O)
dm_thin_pool dm_bio_prison dm_persistent_data hal_netlink(O) coretemp
r8152 usbnet mii igb e1000e(O) mpt3sas mpt2sas scsi_transport_sas
raid_class uas usb_storage xhci_pci xhci_hcd usblp uhci_hcd ehci_pci
ehci_hcd [last unloaded: fbdisk]
<4>[ 8415.348888] CPU: 0 PID: 4611 Comm: md1_raid5 Tainted: P     U
 O   3.19.8 #1
<4>[ 8415.356137] Hardware name: Default string Default string/SKYBAY,
BIOS QX80AR20 06/07/2016
<4>[ 8415.364249] task: ffff880847616210 ti: ffff880831950000 task.ti:
ffff880831950000
<4>[ 8415.371667] RIP: 0010:[<ffffffff813c4446>]  [<ffffffff813c4446>]
memcpy+0x6/0x110
<4>[ 8415.379126] RSP: 0000:ffff880831953990  EFLAGS: 00210206
<4>[ 8415.384398] RAX: ffff880832e87000 RBX: ffff880831953a18 RCX:
0000000000001000
<4>[ 8415.391475] RDX: 0000000000001000 RSI: 0845080000067000 RDI:
ffff880832e87000
<4>[ 8415.398553] RBP: ffff8808319539c8 R08: 0000000000001000 R09:
ffff880831953a18
<4>[ 8415.405660] R10: 0000000000000001 R11: 0000000000000001 R12:
ffffea0020cba1c0
<4>[ 8415.412750] R13: 0000000000000000 R14: 002100000000000e R15:
0000000000067000
<4>[ 8415.419829] FS:  0000000000000000(0000)
GS:ffff88086dc00000(0000) knlGS:0000000000000000
<4>[ 8415.427852] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 8415.433550] CR2: 0000000008120480 CR3: 0000000001c7d000 CR4:
00000000003407f0
<4>[ 8415.440648] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
<4>[ 8415.447748] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
<4>[ 8415.454823] Stack:
<4>[ 8415.456849]  ffffffff8138408f 0000000000001000 0000000000080000
0000000000080000
<4>[ 8415.464283]  0000000000000000 0000000000000000 0000000000001000
ffff880831953a78
<4>[ 8415.471720]  ffffffff8169911e ffff880831953a18 ffffffff8138a5b7
ffff880831953a18
<4>[ 8415.479144] Call Trace:
<4>[ 8415.481578]  [<ffffffff8138408f>] ? async_memcpy+0x9f/0x100
<4>[ 8415.487122]  [<ffffffff8169911e>] async_copy_data+0x12e/0x260
<4>[ 8415.492824]  [<ffffffff8138a5b7>] ? bio_attempt_front_merge+0xb7/0xf0
<4>[ 8415.499214]  [<ffffffff81699bb6>] raid_run_ops+0x876/0x1190
<4>[ 8415.504741]  [<ffffffff8138c5b3>] ? generic_make_request+0xa3/0xf0
<4>[ 8415.510872]  [<ffffffff8169f716>] ? ops_run_io+0x36/0x820
<4>[ 8415.516233]  [<ffffffff81085763>] ? __wake_up+0x53/0x70
<4>[ 8415.521417]  [<ffffffff816a0e7b>] handle_stripe+0xa8b/0x1b50
<4>[ 8415.527031]  [<ffffffff81085206>] ? __wake_up_common+0x16/0x90
<4>[ 8415.532816]  [<ffffffff816a22da>] handle_active_stripes+0x39a/0x450
<4>[ 8415.539035]  [<ffffffff816a2849>] raid5d+0x399/0x630
<4>[ 8415.543964]  [<ffffffff816ac4ed>] md_thread+0x7d/0x130
<4>[ 8415.549064]  [<ffffffff81085370>] ? woken_wake_function+0x20/0x20
<4>[ 8415.555107]  [<ffffffff816ac470>] ? errors_store+0x70/0x70
<4>[ 8415.560552]  [<ffffffff8106a263>] kthread+0xe3/0xf0
<4>[ 8415.565411]  [<ffffffff8106a180>] ? kthreadd+0x160/0x160
<4>[ 8415.570682]  [<ffffffff8192d048>] ret_from_fork+0x58/0x90
<4>[ 8415.576044]  [<ffffffff8106a180>] ? kthreadd+0x160/0x160
<4>[ 8415.581315] Code: 24 4c 8b 64 24 08 c9 c3 e8 68 f9 ff ff 41 80
7c 24 05 00 75 d3 eb e4 90 90 90 90 90 90 90 90 90 90 90 90 90 90 48
89 f8 48 89 d1 <f3> a4 c3 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 20 4c 8b
06 4c 8b
<1>[ 8415.601660] RIP  [<ffffffff813c4446>] memcpy+0x6/0x110
<4>[ 8415.606788]  RSP <ffff880831953990>
<4>[ 8415.612291] ---[ end trace 962cfd98d43b82fa ]---
<4>[ 8415.620528] ------------[ cut here ]------------

^ permalink raw reply

* Re: [PATCH v3 2/3] bcache: update document info
From: wangyijing @ 2016-07-04  6:43 UTC (permalink / raw)
  To: Coly Li, axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel
In-Reply-To: <489c5709-29d0-077c-d8b7-92f12447fc08@coly.li>



在 2016/7/4 13:49, Coly Li 写道:
> 在 16/7/4 上午9:23, Yijing Wang 写道:
>> There is no return in continue_at(), update the documentation.
>>
> 
> Thanks.
> 
> Acked-by: Coly Li <colyli@suse.de>

Thanks very much!

> 
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  drivers/md/bcache/closure.c |    2 +-
>>  drivers/md/bcache/closure.h |    3 ++-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
>> index 9eaf1d6..864e673 100644
>> --- a/drivers/md/bcache/closure.c
>> +++ b/drivers/md/bcache/closure.c
>> @@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
>>  EXPORT_SYMBOL(closure_wait);
>>  
>>  /**
>> - * closure_sync - sleep until a closure a closure has nothing left to wait on
>> + * closure_sync - sleep until a closure has nothing left to wait on
>>   *
>>   * Sleeps until the refcount hits 1 - the thread that's running the closure owns
>>   * the last refcount.
>> diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
>> index 782cc2c..9b2fe2d 100644
>> --- a/drivers/md/bcache/closure.h
>> +++ b/drivers/md/bcache/closure.h
>> @@ -31,7 +31,8 @@
>>   * passing it, as you might expect, the function to run when nothing is pending
>>   * and the workqueue to run that function out of.
>>   *
>> - * continue_at() also, critically, is a macro that returns the calling function.
>> + * continue_at() also, critically, requires a 'return' immediately following the
>> + * location where this macro is referenced, to return to the calling function.
>>   * There's good reason for this.
>>   *
>>   * To use safely closures asynchronously, they must always have a refcount while
>>
> 
> 

^ permalink raw reply

* Re: URE, link resets, user hostile defaults
From: Hannes Reinecke @ 2016-07-04  6:00 UTC (permalink / raw)
  To: Chris Murphy, Edward Kuns; +Cc: Zygo Blaxell, Linux-RAID
In-Reply-To: <CAJCQCtT2Sxi1GgmNaO8i46LuELpA_nrWLnVzAQMz7Qx_mdwXvw@mail.gmail.com>

On 07/01/2016 10:43 PM, Chris Murphy wrote:
> Here's a fun one of these I just got off the Fedora users mailing list
> with a laptop drive that's apparently hanging on *write*. This I would
> not expect to take a long time for a drive to figure out, but... there
> are more resets than there are write errors, and in fact there's no
> discrete write error from the drive, all we know is the failed command
> is a WRITE command.
> 
> What seems to happen is, everything in the queue gets obliterated in
> the reset, and when ext4 finds out everything failed, not just one
> write, it barfs and goes read only.
> 
> http://pastebin.com/3JAL297z
> 
> How might this turn out differently if the drive reported a single
> discrete write error? I don't know how any file system tolerates this
> because it's so rare. Would ext4 just try to write again? Would it try
> to write to the same sector or another one? Or maybe the write finally
> succeeds by resulting in a remap (?) But this sure is dang slow to
> recover from a bad write. I don't understand the engineering rational
> for this. Maybe it's a firmware bug?
> 
> 
Could be. At the very least it's an issue with EH interaction.
ATA COMRESET fails, ie libata EH fails to reset the SATA link.
Which is pretty terminal, so the device is set to offline afterwards.

This is most definitely an ATA issue, and doesn't really belong in this
context.
(Have you reported it on linux-ide?)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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

* Re: [PATCH v3 2/3] bcache: update document info
From: Coly Li @ 2016-07-04  5:49 UTC (permalink / raw)
  To: Yijing Wang, axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel
In-Reply-To: <1467595414-20844-1-git-send-email-wangyijing@huawei.com>

在 16/7/4 上午9:23, Yijing Wang 写道:
> There is no return in continue_at(), update the documentation.
> 

Thanks.

Acked-by: Coly Li <colyli@suse.de>

> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/md/bcache/closure.c |    2 +-
>  drivers/md/bcache/closure.h |    3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
> index 9eaf1d6..864e673 100644
> --- a/drivers/md/bcache/closure.c
> +++ b/drivers/md/bcache/closure.c
> @@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
>  EXPORT_SYMBOL(closure_wait);
>  
>  /**
> - * closure_sync - sleep until a closure a closure has nothing left to wait on
> + * closure_sync - sleep until a closure has nothing left to wait on
>   *
>   * Sleeps until the refcount hits 1 - the thread that's running the closure owns
>   * the last refcount.
> diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
> index 782cc2c..9b2fe2d 100644
> --- a/drivers/md/bcache/closure.h
> +++ b/drivers/md/bcache/closure.h
> @@ -31,7 +31,8 @@
>   * passing it, as you might expect, the function to run when nothing is pending
>   * and the workqueue to run that function out of.
>   *
> - * continue_at() also, critically, is a macro that returns the calling function.
> + * continue_at() also, critically, requires a 'return' immediately following the
> + * location where this macro is referenced, to return to the calling function.
>   * There's good reason for this.
>   *
>   * To use safely closures asynchronously, they must always have a refcount while
> 


-- 
Coly Li

^ permalink raw reply

* [PATCH v3 3/3] bcache: Remove redundant block_size assignment
From: Yijing Wang @ 2016-07-04  1:23 UTC (permalink / raw)
  To: axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel,
	Yijing Wang

We have assigned sb->block_size before the switch,
so remove the redundant one.

Reviewed-by: Coly Li <colyli@suse.de>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Acked-by: Eric Wheeler <bcache@lists.ewheeler.net>
---
 drivers/md/bcache/super.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index aecaace..bf4b100 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -134,7 +134,6 @@ static const char *read_super(struct cache_sb *sb, struct block_device *bdev,
 	case BCACHE_SB_VERSION_CDEV:
 	case BCACHE_SB_VERSION_CDEV_WITH_UUID:
 		sb->nbuckets	= le64_to_cpu(s->nbuckets);
-		sb->block_size	= le16_to_cpu(s->block_size);
 		sb->bucket_size	= le16_to_cpu(s->bucket_size);
 
 		sb->nr_in_set	= le16_to_cpu(s->nr_in_set);
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 2/3] bcache: update document info
From: Yijing Wang @ 2016-07-04  1:23 UTC (permalink / raw)
  To: axboe, Kent Overstreet
  Cc: Eric Wheeler, Coly Li, linux-bcache, linux-raid, linux-kernel,
	Yijing Wang

There is no return in continue_at(), update the documentation.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/md/bcache/closure.c |    2 +-
 drivers/md/bcache/closure.h |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
index 9eaf1d6..864e673 100644
--- a/drivers/md/bcache/closure.c
+++ b/drivers/md/bcache/closure.c
@@ -112,7 +112,7 @@ bool closure_wait(struct closure_waitlist *waitlist, struct closure *cl)
 EXPORT_SYMBOL(closure_wait);
 
 /**
- * closure_sync - sleep until a closure a closure has nothing left to wait on
+ * closure_sync - sleep until a closure has nothing left to wait on
  *
  * Sleeps until the refcount hits 1 - the thread that's running the closure owns
  * the last refcount.
diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
index 782cc2c..9b2fe2d 100644
--- a/drivers/md/bcache/closure.h
+++ b/drivers/md/bcache/closure.h
@@ -31,7 +31,8 @@
  * passing it, as you might expect, the function to run when nothing is pending
  * and the workqueue to run that function out of.
  *
- * continue_at() also, critically, is a macro that returns the calling function.
+ * continue_at() also, critically, requires a 'return' immediately following the
+ * location where this macro is referenced, to return to the calling function.
  * There's good reason for this.
  *
  * To use safely closures asynchronously, they must always have a refcount while
-- 
1.7.1

^ 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