* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-09 22:16 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <20170809202125.5zs4h45luqlub5if@kernel.org>
On 08/09/2017 03:21 PM, Shaohua Li wrote:
> On Wed, Aug 09, 2017 at 10:35:39AM -0500, Goldwyn Rodrigues wrote:
>>
>>
>> On 08/09/2017 10:02 AM, Shaohua Li wrote:
>>> On Wed, Aug 09, 2017 at 06:44:55AM -0500, Goldwyn Rodrigues wrote:
>>>>
>>>>
>>>> On 08/08/2017 03:32 PM, Shaohua Li wrote:
>>>>> On Wed, Jul 26, 2017 at 06:57:58PM -0500, Goldwyn Rodrigues wrote:
>>>>>> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>>>>>
>>>>>> Nowait is a feature of direct AIO, where users can request
>>>>>> to return immediately if the I/O is going to block. This translates
>>>>>> to REQ_NOWAIT in bio.bi_opf flags. While request based devices
>>>>>> don't wait, stacked devices such as md/dm will.
>>>>>>
>>>>>> In order to explicitly mark stacked devices as supported, we
>>>>>> set the QUEUE_FLAG_NOWAIT in the queue_flags and return -EAGAIN
>>>>>> whenever the device would block.
>>>>>
>>>>> probably you should route this patch to Jens first, DM/MD are different trees.
>>>>
>>>> Yes, I have sent it to linux-block as well, and he has commented as well.
>>>>
>>>>
>>>>>
>>>>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>>>>> ---
>>>>>> block/blk-core.c | 3 ++-
>>>>>> include/linux/blkdev.h | 2 ++
>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>>>>> index 970b9c9638c5..1c9a981d88e5 100644
>>>>>> --- a/block/blk-core.c
>>>>>> +++ b/block/blk-core.c
>>>>>> @@ -2025,7 +2025,8 @@ generic_make_request_checks(struct bio *bio)
>>>>>> * if queue is not a request based queue.
>>>>>> */
>>>>>>
>>>>>> - if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q))
>>>>>> + if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q) &&
>>>>>> + !blk_queue_supports_nowait(q))
>>>>>> goto not_supported;
>>>>>>
>>>>>> part = bio->bi_bdev->bd_part;
>>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>>>> --- a/include/linux/blkdev.h
>>>>>> +++ b/include/linux/blkdev.h
>>>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>>>>>
>>>>>> #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
>>>>>> (1 << QUEUE_FLAG_STACKABLE) | \
>>>>>> @@ -732,6 +733,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
>>>>>> #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
>>>>>> #define blk_queue_scsi_passthrough(q) \
>>>>>> test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags)
>>>>>> +#define blk_queue_supports_nowait(q) test_bit(QUEUE_FLAG_NOWAIT, &(q)->queue_flags)
>>>>>
>>>>> Should this bit consider under layer disks? For example, one raid array disk
>>>>> doesn't support NOWAIT, shouldn't we disable NOWAIT for the array?
>>>>
>>>> Yes, it should. I will add a check before setting the flag. Thanks.
>>>> Request-based devices don't wait. So, they would not have this flag set.
>>>> It is only the bio-based, with the make_request_fn hook which need this.
>>>>
>>>>>
>>>>> I have another generic question. If a bio is splitted into 2 bios, one bio
>>>>> doesn't need to wait but the other need to wait. We will return -EAGAIN for the
>>>>> second bio, so the whole bio will return -EAGAIN, but the first bio is already
>>>>> dispatched to disk. Is this correct behavior?
>>>>>
>>>>
>>>> No, from a multi-device point of view, this is inconsistent. I have
>>>> tried the request bio returns -EAGAIN before the split, but I shall
>>>> check again. Where do you see this happening?
>>>
>>> No, this isn't multi-device specific, any driver can do it. Please see blk_queue_split.
>>>
>>
>> In that case, the bio end_io function is chained and the bio of the
>> split will replicate the error to the parent (if not already set).
>
> this doesn't answer my question. So if a bio returns -EAGAIN, part of the bio
> probably already dispatched to disk (if the bio is splitted to 2 bios, one
> returns -EAGAIN, the other one doesn't block and dispatch to disk), what will
> application be going to do? I think this is different to other IO errors. FOr
> other IO errors, application will handle the error, while we ask app to retry
> the whole bio here and app doesn't know part of bio is already written to disk.
It is the same as for other I/O errors as well, such as EIO. You do not
know which bio of all submitted bio's returned the error EIO. The
application would and should consider the whole I/O as failed.
The user application does not know of bios, or how it is going to be
split in the underlying layers. It knows at the system call level. In
this case, the EAGAIN will be returned to the user for the whole I/O not
as a part of the I/O. It is up to application to try the I/O again with
or without RWF_NOWAIT set. In direct I/O, it is bubbled out using
dio->io_error. You can read about it at the patch header for the initial
patchset at [1].
Use case: It is for applications having two threads, a compute thread
and an I/O thread. It would try to push AIO as much as possible in the
compute thread using RWF_NOWAIT, and if it fails, would pass it on to
I/O thread which would perform without RWF_NOWAIT. End result if done
right is you save on context switches and all the
synchronization/messaging machinery to perform I/O.
[1] http://marc.info/?l=linux-block&m=149789003305876&w=2
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Shaohua Li @ 2017-08-10 1:17 UTC (permalink / raw)
To: Goldwyn Rodrigues; +Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <b3017adc-619d-06f7-ffb5-5ba14f01ffc2@suse.de>
On Wed, Aug 09, 2017 at 05:16:23PM -0500, Goldwyn Rodrigues wrote:
>
>
> On 08/09/2017 03:21 PM, Shaohua Li wrote:
> > On Wed, Aug 09, 2017 at 10:35:39AM -0500, Goldwyn Rodrigues wrote:
> >>
> >>
> >> On 08/09/2017 10:02 AM, Shaohua Li wrote:
> >>> On Wed, Aug 09, 2017 at 06:44:55AM -0500, Goldwyn Rodrigues wrote:
> >>>>
> >>>>
> >>>> On 08/08/2017 03:32 PM, Shaohua Li wrote:
> >>>>> On Wed, Jul 26, 2017 at 06:57:58PM -0500, Goldwyn Rodrigues wrote:
> >>>>>> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >>>>>>
> >>>>>> Nowait is a feature of direct AIO, where users can request
> >>>>>> to return immediately if the I/O is going to block. This translates
> >>>>>> to REQ_NOWAIT in bio.bi_opf flags. While request based devices
> >>>>>> don't wait, stacked devices such as md/dm will.
> >>>>>>
> >>>>>> In order to explicitly mark stacked devices as supported, we
> >>>>>> set the QUEUE_FLAG_NOWAIT in the queue_flags and return -EAGAIN
> >>>>>> whenever the device would block.
> >>>>>
> >>>>> probably you should route this patch to Jens first, DM/MD are different trees.
> >>>>
> >>>> Yes, I have sent it to linux-block as well, and he has commented as well.
> >>>>
> >>>>
> >>>>>
> >>>>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >>>>>> ---
> >>>>>> block/blk-core.c | 3 ++-
> >>>>>> include/linux/blkdev.h | 2 ++
> >>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/block/blk-core.c b/block/blk-core.c
> >>>>>> index 970b9c9638c5..1c9a981d88e5 100644
> >>>>>> --- a/block/blk-core.c
> >>>>>> +++ b/block/blk-core.c
> >>>>>> @@ -2025,7 +2025,8 @@ generic_make_request_checks(struct bio *bio)
> >>>>>> * if queue is not a request based queue.
> >>>>>> */
> >>>>>>
> >>>>>> - if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q))
> >>>>>> + if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q) &&
> >>>>>> + !blk_queue_supports_nowait(q))
> >>>>>> goto not_supported;
> >>>>>>
> >>>>>> part = bio->bi_bdev->bd_part;
> >>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> >>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
> >>>>>> --- a/include/linux/blkdev.h
> >>>>>> +++ b/include/linux/blkdev.h
> >>>>>> @@ -633,6 +633,7 @@ struct request_queue {
> >>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
> >>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
> >>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
> >>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
> >>>>>>
> >>>>>> #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
> >>>>>> (1 << QUEUE_FLAG_STACKABLE) | \
> >>>>>> @@ -732,6 +733,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
> >>>>>> #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
> >>>>>> #define blk_queue_scsi_passthrough(q) \
> >>>>>> test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags)
> >>>>>> +#define blk_queue_supports_nowait(q) test_bit(QUEUE_FLAG_NOWAIT, &(q)->queue_flags)
> >>>>>
> >>>>> Should this bit consider under layer disks? For example, one raid array disk
> >>>>> doesn't support NOWAIT, shouldn't we disable NOWAIT for the array?
> >>>>
> >>>> Yes, it should. I will add a check before setting the flag. Thanks.
> >>>> Request-based devices don't wait. So, they would not have this flag set.
> >>>> It is only the bio-based, with the make_request_fn hook which need this.
> >>>>
> >>>>>
> >>>>> I have another generic question. If a bio is splitted into 2 bios, one bio
> >>>>> doesn't need to wait but the other need to wait. We will return -EAGAIN for the
> >>>>> second bio, so the whole bio will return -EAGAIN, but the first bio is already
> >>>>> dispatched to disk. Is this correct behavior?
> >>>>>
> >>>>
> >>>> No, from a multi-device point of view, this is inconsistent. I have
> >>>> tried the request bio returns -EAGAIN before the split, but I shall
> >>>> check again. Where do you see this happening?
> >>>
> >>> No, this isn't multi-device specific, any driver can do it. Please see blk_queue_split.
> >>>
> >>
> >> In that case, the bio end_io function is chained and the bio of the
> >> split will replicate the error to the parent (if not already set).
> >
> > this doesn't answer my question. So if a bio returns -EAGAIN, part of the bio
> > probably already dispatched to disk (if the bio is splitted to 2 bios, one
> > returns -EAGAIN, the other one doesn't block and dispatch to disk), what will
> > application be going to do? I think this is different to other IO errors. FOr
> > other IO errors, application will handle the error, while we ask app to retry
> > the whole bio here and app doesn't know part of bio is already written to disk.
>
> It is the same as for other I/O errors as well, such as EIO. You do not
> know which bio of all submitted bio's returned the error EIO. The
> application would and should consider the whole I/O as failed.
>
> The user application does not know of bios, or how it is going to be
> split in the underlying layers. It knows at the system call level. In
> this case, the EAGAIN will be returned to the user for the whole I/O not
> as a part of the I/O. It is up to application to try the I/O again with
> or without RWF_NOWAIT set. In direct I/O, it is bubbled out using
> dio->io_error. You can read about it at the patch header for the initial
> patchset at [1].
>
> Use case: It is for applications having two threads, a compute thread
> and an I/O thread. It would try to push AIO as much as possible in the
> compute thread using RWF_NOWAIT, and if it fails, would pass it on to
> I/O thread which would perform without RWF_NOWAIT. End result if done
> right is you save on context switches and all the
> synchronization/messaging machinery to perform I/O.
>
> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
Yes, I knew the concept, but I didn't see previous patches mentioned the
-EAGAIN actually should be taken as a real IO error. This means a lot to
applications and make the API hard to use. I'm wondering if we should disable
bio split for NOWAIT bio, which will make the -EAGAIN only mean 'try again'.
Thanks,
Shaohua
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-10 2:07 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <20170810011742.s45ugh55jslvkguu@kernel.org>
On 08/09/2017 08:17 PM, Shaohua Li wrote:
> On Wed, Aug 09, 2017 at 05:16:23PM -0500, Goldwyn Rodrigues wrote:
>>
>>
>> On 08/09/2017 03:21 PM, Shaohua Li wrote:
>>> On Wed, Aug 09, 2017 at 10:35:39AM -0500, Goldwyn Rodrigues wrote:
>>>>
>>>>
>>>> On 08/09/2017 10:02 AM, Shaohua Li wrote:
>>>>> On Wed, Aug 09, 2017 at 06:44:55AM -0500, Goldwyn Rodrigues wrote:
>>>>>>
>>>>>>
>>>>>> On 08/08/2017 03:32 PM, Shaohua Li wrote:
>>>>>>> On Wed, Jul 26, 2017 at 06:57:58PM -0500, Goldwyn Rodrigues wrote:
>>>>>>>> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>>>>>>>
>>>>>>>> Nowait is a feature of direct AIO, where users can request
>>>>>>>> to return immediately if the I/O is going to block. This translates
>>>>>>>> to REQ_NOWAIT in bio.bi_opf flags. While request based devices
>>>>>>>> don't wait, stacked devices such as md/dm will.
>>>>>>>>
>>>>>>>> In order to explicitly mark stacked devices as supported, we
>>>>>>>> set the QUEUE_FLAG_NOWAIT in the queue_flags and return -EAGAIN
>>>>>>>> whenever the device would block.
>>>>>>>
>>>>>>> probably you should route this patch to Jens first, DM/MD are different trees.
>>>>>>
>>>>>> Yes, I have sent it to linux-block as well, and he has commented as well.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>>>>>>> ---
>>>>>>>> block/blk-core.c | 3 ++-
>>>>>>>> include/linux/blkdev.h | 2 ++
>>>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>>>>>>> index 970b9c9638c5..1c9a981d88e5 100644
>>>>>>>> --- a/block/blk-core.c
>>>>>>>> +++ b/block/blk-core.c
>>>>>>>> @@ -2025,7 +2025,8 @@ generic_make_request_checks(struct bio *bio)
>>>>>>>> * if queue is not a request based queue.
>>>>>>>> */
>>>>>>>>
>>>>>>>> - if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q))
>>>>>>>> + if ((bio->bi_opf & REQ_NOWAIT) && !queue_is_rq_based(q) &&
>>>>>>>> + !blk_queue_supports_nowait(q))
>>>>>>>> goto not_supported;
>>>>>>>>
>>>>>>>> part = bio->bi_bdev->bd_part;
>>>>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>>>>>> --- a/include/linux/blkdev.h
>>>>>>>> +++ b/include/linux/blkdev.h
>>>>>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>>>>>>>
>>>>>>>> #define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
>>>>>>>> (1 << QUEUE_FLAG_STACKABLE) | \
>>>>>>>> @@ -732,6 +733,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
>>>>>>>> #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
>>>>>>>> #define blk_queue_scsi_passthrough(q) \
>>>>>>>> test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags)
>>>>>>>> +#define blk_queue_supports_nowait(q) test_bit(QUEUE_FLAG_NOWAIT, &(q)->queue_flags)
>>>>>>>
>>>>>>> Should this bit consider under layer disks? For example, one raid array disk
>>>>>>> doesn't support NOWAIT, shouldn't we disable NOWAIT for the array?
>>>>>>
>>>>>> Yes, it should. I will add a check before setting the flag. Thanks.
>>>>>> Request-based devices don't wait. So, they would not have this flag set.
>>>>>> It is only the bio-based, with the make_request_fn hook which need this.
>>>>>>
>>>>>>>
>>>>>>> I have another generic question. If a bio is splitted into 2 bios, one bio
>>>>>>> doesn't need to wait but the other need to wait. We will return -EAGAIN for the
>>>>>>> second bio, so the whole bio will return -EAGAIN, but the first bio is already
>>>>>>> dispatched to disk. Is this correct behavior?
>>>>>>>
>>>>>>
>>>>>> No, from a multi-device point of view, this is inconsistent. I have
>>>>>> tried the request bio returns -EAGAIN before the split, but I shall
>>>>>> check again. Where do you see this happening?
>>>>>
>>>>> No, this isn't multi-device specific, any driver can do it. Please see blk_queue_split.
>>>>>
>>>>
>>>> In that case, the bio end_io function is chained and the bio of the
>>>> split will replicate the error to the parent (if not already set).
>>>
>>> this doesn't answer my question. So if a bio returns -EAGAIN, part of the bio
>>> probably already dispatched to disk (if the bio is splitted to 2 bios, one
>>> returns -EAGAIN, the other one doesn't block and dispatch to disk), what will
>>> application be going to do? I think this is different to other IO errors. FOr
>>> other IO errors, application will handle the error, while we ask app to retry
>>> the whole bio here and app doesn't know part of bio is already written to disk.
>>
>> It is the same as for other I/O errors as well, such as EIO. You do not
>> know which bio of all submitted bio's returned the error EIO. The
>> application would and should consider the whole I/O as failed.
>>
>> The user application does not know of bios, or how it is going to be
>> split in the underlying layers. It knows at the system call level. In
>> this case, the EAGAIN will be returned to the user for the whole I/O not
>> as a part of the I/O. It is up to application to try the I/O again with
>> or without RWF_NOWAIT set. In direct I/O, it is bubbled out using
>> dio->io_error. You can read about it at the patch header for the initial
>> patchset at [1].
>>
>> Use case: It is for applications having two threads, a compute thread
>> and an I/O thread. It would try to push AIO as much as possible in the
>> compute thread using RWF_NOWAIT, and if it fails, would pass it on to
>> I/O thread which would perform without RWF_NOWAIT. End result if done
>> right is you save on context switches and all the
>> synchronization/messaging machinery to perform I/O.
>>
>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>
> Yes, I knew the concept, but I didn't see previous patches mentioned the
> -EAGAIN actually should be taken as a real IO error. This means a lot to
> applications and make the API hard to use. I'm wondering if we should disable
> bio split for NOWAIT bio, which will make the -EAGAIN only mean 'try again'.
>
Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say the
API is hard to use? Do you have a case to back it up?
No, not splitting the bio does not make sense here. I do not see any
advantage in it, unless you can present a case otherwise.
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 2:17 UTC (permalink / raw)
To: Goldwyn Rodrigues, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <a20baa0e-261b-f912-5bca-40c7523d7c44@suse.de>
On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>> I shall check again. Where do you see this happening?
>>>>>>
>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>> Please see blk_queue_split.
>>>>>>
>>>>>
>>>>> In that case, the bio end_io function is chained and the bio of
>>>>> the split will replicate the error to the parent (if not already
>>>>> set).
>>>>
>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>> of the bio probably already dispatched to disk (if the bio is
>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>> block and dispatch to disk), what will application be going to do?
>>>> I think this is different to other IO errors. FOr other IO errors,
>>>> application will handle the error, while we ask app to retry the
>>>> whole bio here and app doesn't know part of bio is already written
>>>> to disk.
>>>
>>> It is the same as for other I/O errors as well, such as EIO. You do
>>> not know which bio of all submitted bio's returned the error EIO.
>>> The application would and should consider the whole I/O as failed.
>>>
>>> The user application does not know of bios, or how it is going to be
>>> split in the underlying layers. It knows at the system call level.
>>> In this case, the EAGAIN will be returned to the user for the whole
>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>> out using dio->io_error. You can read about it at the patch header
>>> for the initial patchset at [1].
>>>
>>> Use case: It is for applications having two threads, a compute
>>> thread and an I/O thread. It would try to push AIO as much as
>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>> would pass it on to I/O thread which would perform without
>>> RWF_NOWAIT. End result if done right is you save on context switches
>>> and all the synchronization/messaging machinery to perform I/O.
>>>
>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>
>> Yes, I knew the concept, but I didn't see previous patches mentioned
>> the -EAGAIN actually should be taken as a real IO error. This means a
>> lot to applications and make the API hard to use. I'm wondering if we
>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>> only mean 'try again'.
>
> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
> the API is hard to use? Do you have a case to back it up?
Because it is hard to use, and potentially suboptimal. Let's say you're
doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
short write, or do we return EWOULDBLOCK? If the latter, then that
really sucks from an API point of view.
> No, not splitting the bio does not make sense here. I do not see any
> advantage in it, unless you can present a case otherwise.
It ties back into the "hard to use" that I do agree with IFF we don't
return the short write. It's hard for an application to use that
efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
full 1MB from a different context.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 2:18 UTC (permalink / raw)
To: Shaohua Li, Goldwyn Rodrigues
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <10dbe9f0-e2a0-456f-b271-7cf0312aa1d5@kernel.dk>
On 08/08/2017 02:36 PM, Jens Axboe wrote:
> On 08/08/2017 02:32 PM, Shaohua Li wrote:
>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>> --- a/include/linux/blkdev.h
>>> +++ b/include/linux/blkdev.h
>>> @@ -633,6 +633,7 @@ struct request_queue {
>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>
> Does this work on 32-bit, where sizeof(unsigned long) == 32?
I didn't get an answer to this one.
--
Jens Axboe
^ permalink raw reply
* [PATCH] raid5: remove raid5_build_block
From: Guoqing Jiang @ 2017-08-10 8:12 UTC (permalink / raw)
To: shli; +Cc: linux-raid, Guoqing Jiang
Now raid5_build_block is just called to set the
sector of r5dev, raid5_compute_blocknr can be
used directly for the purpose.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/raid5.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 356cd9c..41143f9 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -491,7 +491,6 @@ static int grow_buffers(struct stripe_head *sh, gfp_t gfp)
return 0;
}
-static void raid5_build_block(struct stripe_head *sh, int i, int previous);
static void stripe_set_idx(sector_t stripe, struct r5conf *conf, int previous,
struct stripe_head *sh);
@@ -527,7 +526,7 @@ static void init_stripe(struct stripe_head *sh, sector_t sector, int previous)
WARN_ON(1);
}
dev->flags = 0;
- raid5_build_block(sh, i, previous);
+ dev->sector = raid5_compute_blocknr(sh, i, previous);
}
if (read_seqcount_retry(&conf->gen_lock, seq))
goto retry;
@@ -2663,14 +2662,6 @@ static void raid5_end_write_request(struct bio *bi)
raid5_release_stripe(sh->batch_head);
}
-static void raid5_build_block(struct stripe_head *sh, int i, int previous)
-{
- struct r5dev *dev = &sh->dev[i];
-
- dev->flags = 0;
- dev->sector = raid5_compute_blocknr(sh, i, previous);
-}
-
static void raid5_error(struct mddev *mddev, struct md_rdev *rdev)
{
char b[BDEVNAME_SIZE];
--
2.10.0
^ permalink raw reply related
* [PATCH v2] mdopen: prevent named arrays devices from buffer overflow
From: jcejka @ 2017-08-10 9:32 UTC (permalink / raw)
To: linux-raid; +Cc: Josef Cejka
In-Reply-To: <20170803164816.29998-1-jcejka@suse.com>
From: Josef Cejka <jcejka@suse.com>
Device names for named arrays must fit into 32 bytes
and longer strings provided by user should be rejected. Now they
corrupt the stack (overwrite following devname[] buffer) and
(if not detected) create arrays using old create_on_open
mechanism because write to new_array fails with E2BIG.
Reproducer:
echo "CREATE names=yes" >>/etc/mdadm.conf
mdadm -A /dev/md/abcdefghijklmnopqrstuvwxyz123 --uuid=...
Signed-off-by: Josef Cejka <jcejka@suse.com>
Reviewed-by: Coly Li <colyli@suse.de>
---
mdopen.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/mdopen.c b/mdopen.c
index 0f3a244..fd8a1db 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -314,7 +314,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (num < 0 && cname && ci->names) {
int fd;
int n = -1;
- sprintf(devnm, "md_%s", cname);
+ if (snprintf(devnm, sizeof(devnm), "md_%s", cname) >= sizeof(devnm)) {
+ pr_err("Device name md_%s must be shorter than %d bytes.\n", cname, sizeof(devnm));
+ return -1;
+ }
if (block_udev)
udev_block(devnm);
fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
@@ -364,7 +367,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
udev_block(devnm);
}
- sprintf(devname, "/dev/%s", devnm);
+ if (snprintf(devname, sizeof(devname), "/dev/%s", devnm) >= sizeof(devname)) {
+ pr_err("Device path /dev/%s must be shorter than %d bytes.\n", devnm, sizeof(devname));
+ return -1;
+ }
if (dev && dev[0] == '/')
strcpy(chosen, dev);
--
2.12.3
^ permalink raw reply related
* [PATCH v2] mdadm: allow to assemble /dev/md_xxx named arrays.
From: jcejka @ 2017-08-10 9:41 UTC (permalink / raw)
To: linux-raid; +Cc: Josef Cejka
In-Reply-To: <20170803163236.28958-1-jcejka@suse.com>
From: Josef Cejka <jcejka@suse.com>
Allow to assemble arrays using /dev/md_xxx name format
both from cmdline like mdadm -A /dev/md_xxx
or using ASSEMBLE directive in mdadm.conf
Signed-off-by: Josef Cejka <jcejka@suse.com>
Reviewed-by: Coly Li <colyli@suse.de>
---
config.c | 5 +++--
mdopen.c | 26 ++++++++++++++++----------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/config.c b/config.c
index 48e0278..113ef01 100644
--- a/config.c
+++ b/config.c
@@ -389,6 +389,7 @@ void arrayline(char *line)
* /dev/md/{anything}
* /dev/mdNN
* /dev/md_dNN
+ * /dev/md_XXX
* <ignore>
* or anything that doesn't start '/' or '<'
*/
@@ -397,8 +398,8 @@ void arrayline(char *line)
(w[0] != '/' && w[0] != '<') ||
(strncmp(w, "/dev/md", 7) == 0 &&
is_number(w + 7)) ||
- (strncmp(w, "/dev/md_d", 9) == 0 &&
- is_number(w + 9))) {
+ (strncmp(w, "/dev/md_", 8) == 0 &&
+ strlen(w) > 8)) {
/* This is acceptable */;
if (mis.devname)
pr_err("only give one device per ARRAY line: %s and %s\n",
diff --git a/mdopen.c b/mdopen.c
index c4f1c12..0f3a244 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -174,19 +174,25 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
num = strtoul(e, NULL, 10);
strcpy(cname, dev+5);
cname[e-(dev+5)] = 0;
- /* name *must* be mdXX or md_dXX in this context */
+ /* name is not mdXX or md_dXX */
if (num < 0 ||
(strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
- pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
- dev, dev+5);
- return -1;
+ if (strncmp(cname, "md_", 3) != 0) {
+ pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
+ dev, dev+5);
+ return -1;
+ }
+ /* /dev/md_<string> format */
+ strcpy(cname, dev+8);
+ num = -1;
+ } else {
+ if (strcmp(cname, "md") == 0)
+ use_mdp = 0;
+ else
+ use_mdp = 1;
+ /* recreate name: /dev/md/0 or /dev/md/d0 */
+ sprintf(cname, "%s%d", use_mdp?"d":"", num);
}
- if (strcmp(cname, "md") == 0)
- use_mdp = 0;
- else
- use_mdp = 1;
- /* recreate name: /dev/md/0 or /dev/md/d0 */
- sprintf(cname, "%s%d", use_mdp?"d":"", num);
} else
strcpy(cname, dev);
--
2.12.3
^ permalink raw reply related
* [PATCH] Detail: correct output for active arrays
From: Mariusz Tkaczyk @ 2017-08-10 9:43 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Mariusz Tkaczyk
The check for inactive array is incorrect as it compares it against
active array. Introduce a new function md_is_array_active so the check
is consistent across the code.
As the output contains list of disks in the array include this
information in sysfs read.
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
---
Detail.c | 15 +++++++--------
mdadm.h | 1 +
util.c | 15 +++++++++------
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/Detail.c b/Detail.c
index 2332b85..2c9fb24 100644
--- a/Detail.c
+++ b/Detail.c
@@ -86,7 +86,8 @@ int Detail(char *dev, struct context *c)
dev, strerror(errno));
return rv;
}
- sra = sysfs_read(fd, NULL, GET_VERSION | GET_DEVS | GET_ARRAY_STATE);
+ sra = sysfs_read(fd, NULL, GET_VERSION | GET_DEVS |
+ GET_ARRAY_STATE | GET_STATE);
if (!sra) {
if (md_get_array_info(fd, &array)) {
pr_err("%s does not appear to be an md device\n", dev);
@@ -96,8 +97,7 @@ int Detail(char *dev, struct context *c)
}
external = (sra != NULL && sra->array.major_version == -1 &&
sra->array.minor_version == -2);
- inactive = (sra->array_state == ARRAY_ACTIVE ||
- sra->array_state == ARRAY_CLEAR);
+ inactive = (sra != NULL && !md_array_is_active(sra));
st = super_by_fd(fd, &subarray);
if (md_get_array_info(fd, &array)) {
if (errno == ENODEV) {
@@ -314,11 +314,10 @@ int Detail(char *dev, struct context *c)
next = array.raid_disks * 2;
if (inactive) {
struct mdinfo *mdi;
- if (sra != NULL)
- for (mdi = sra->devs; mdi; mdi = mdi->next) {
- disks[next++] = mdi->disk;
- disks[next - 1].number = -1;
- }
+ for (mdi = sra->devs; mdi; mdi = mdi->next) {
+ disks[next++] = mdi->disk;
+ disks[next - 1].number = -1;
+ }
} else for (d = 0; d < max_disks; d++) {
mdu_disk_info_t disk;
disk.number = d;
diff --git a/mdadm.h b/mdadm.h
index ee9b837..191ae8f 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1425,6 +1425,7 @@ extern int Restore_metadata(char *dev, char *dir, struct context *c,
int md_array_valid(int fd);
int md_array_active(int fd);
+int md_array_is_active(struct mdinfo *info);
int md_get_array_info(int fd, struct mdu_array_info_s *array);
int md_set_array_info(int fd, struct mdu_array_info_s *array);
int md_get_disk_info(int fd, struct mdu_disk_info_s *disk);
diff --git a/util.c b/util.c
index 8eeb509..c1c8509 100644
--- a/util.c
+++ b/util.c
@@ -228,15 +228,11 @@ int md_array_active(int fd)
{
struct mdinfo *sra;
struct mdu_array_info_s array;
- int ret;
+ int ret = 0;
sra = sysfs_read(fd, NULL, GET_ARRAY_STATE);
if (sra) {
- if (sra->array_state != ARRAY_CLEAR &&
- sra->array_state != ARRAY_INACTIVE &&
- sra->array_state != ARRAY_UNKNOWN_STATE)
- ret = 0;
- else
+ if (!md_array_is_active(sra))
ret = -ENODEV;
free(sra);
@@ -251,6 +247,13 @@ int md_array_active(int fd)
return !ret;
}
+int md_array_is_active(struct mdinfo *info)
+{
+ return (info->array_state != ARRAY_CLEAR &&
+ info->array_state != ARRAY_INACTIVE &&
+ info->array_state != ARRAY_UNKNOWN_STATE);
+}
+
/*
* Get array info from the kernel. Longer term we want to deprecate the
* ioctl and get it from sysfs.
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-10 11:38 UTC (permalink / raw)
To: Jens Axboe, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <284fb7b3-9265-229f-4c66-bd043b94ec9f@kernel.dk>
On 08/09/2017 09:18 PM, Jens Axboe wrote:
> On 08/08/2017 02:36 PM, Jens Axboe wrote:
>> On 08/08/2017 02:32 PM, Shaohua Li wrote:
>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>> --- a/include/linux/blkdev.h
>>>> +++ b/include/linux/blkdev.h
>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>
>> Does this work on 32-bit, where sizeof(unsigned long) == 32?
>
> I didn't get an answer to this one.
>
Oh, I assumed the question is rhetorical.
No, it will not work on 32-bit. I was planning to change the field
queue_flags to u64. Is that okay?
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-10 11:49 UTC (permalink / raw)
To: Jens Axboe, Shaohua Li; +Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <3a23e4af-9ec0-43f3-5aa0-5af68e0dac07@kernel.dk>
On 08/09/2017 09:17 PM, Jens Axboe wrote:
> On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>>> I shall check again. Where do you see this happening?
>>>>>>>
>>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>>> Please see blk_queue_split.
>>>>>>>
>>>>>>
>>>>>> In that case, the bio end_io function is chained and the bio of
>>>>>> the split will replicate the error to the parent (if not already
>>>>>> set).
>>>>>
>>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>>> of the bio probably already dispatched to disk (if the bio is
>>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>>> block and dispatch to disk), what will application be going to do?
>>>>> I think this is different to other IO errors. FOr other IO errors,
>>>>> application will handle the error, while we ask app to retry the
>>>>> whole bio here and app doesn't know part of bio is already written
>>>>> to disk.
>>>>
>>>> It is the same as for other I/O errors as well, such as EIO. You do
>>>> not know which bio of all submitted bio's returned the error EIO.
>>>> The application would and should consider the whole I/O as failed.
>>>>
>>>> The user application does not know of bios, or how it is going to be
>>>> split in the underlying layers. It knows at the system call level.
>>>> In this case, the EAGAIN will be returned to the user for the whole
>>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>>> out using dio->io_error. You can read about it at the patch header
>>>> for the initial patchset at [1].
>>>>
>>>> Use case: It is for applications having two threads, a compute
>>>> thread and an I/O thread. It would try to push AIO as much as
>>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>>> would pass it on to I/O thread which would perform without
>>>> RWF_NOWAIT. End result if done right is you save on context switches
>>>> and all the synchronization/messaging machinery to perform I/O.
>>>>
>>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>>
>>> Yes, I knew the concept, but I didn't see previous patches mentioned
>>> the -EAGAIN actually should be taken as a real IO error. This means a
>>> lot to applications and make the API hard to use. I'm wondering if we
>>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>>> only mean 'try again'.
>>
>> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
>> the API is hard to use? Do you have a case to back it up?
>
> Because it is hard to use, and potentially suboptimal. Let's say you're
> doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
> short write, or do we return EWOULDBLOCK? If the latter, then that
> really sucks from an API point of view.
>
>> No, not splitting the bio does not make sense here. I do not see any
>> advantage in it, unless you can present a case otherwise.
>
> It ties back into the "hard to use" that I do agree with IFF we don't
> return the short write. It's hard for an application to use that
> efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
> full 1MB from a different context.
>
It returns the error code only and not short reads/writes. But isn't
that true for all system calls in case of error?
For aio, there are two result fields in io_event out of which one could
be used for error while the other be used for amount of writes/reads
performed. However, only one is used. This will not work with
pread()/pwrite() calls though because of the limitation of return values.
Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
successful. What short return value should the system call return?
--
Goldwyn
^ permalink raw reply
* Re: [PATCH] mdadm: allow to assemble /dev/md_xxx named arrays.
From: Coly Li @ 2017-08-10 12:24 UTC (permalink / raw)
To: jcejka, linux-raid
In-Reply-To: <20170803163236.28958-1-jcejka@suse.com>
On 2017/8/4 上午12:32, jcejka@suse.com wrote:
> From: Josef Cejka <jcejka@suse.com>
>
> Allow to assemble arrays using /dev/md_xxx name format
> both from cmdline like mdadm -A /dev/md_xxx
> or using ASSEMBLE directive in mdadm.conf
>
> Signed-off-by: Josef Cejka <jcejka@suse.com>
Reviewed-by: Coly Li <colyli@suse.de>
Thanks.
Coly Li
> ---
> config.c | 5 +++--
> mdopen.c | 26 ++++++++++++++++----------
> 2 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/config.c b/config.c
> index 48e0278..113ef01 100644
> --- a/config.c
> +++ b/config.c
> @@ -389,6 +389,7 @@ void arrayline(char *line)
> * /dev/md/{anything}
> * /dev/mdNN
> * /dev/md_dNN
> + * /dev/md_XXX
> * <ignore>
> * or anything that doesn't start '/' or '<'
> */
> @@ -397,8 +398,8 @@ void arrayline(char *line)
> (w[0] != '/' && w[0] != '<') ||
> (strncmp(w, "/dev/md", 7) == 0 &&
> is_number(w + 7)) ||
> - (strncmp(w, "/dev/md_d", 9) == 0 &&
> - is_number(w + 9))) {
> + (strncmp(w, "/dev/md_", 8) == 0 &&
> + strlen(w) > 8)) {
> /* This is acceptable */;
> if (mis.devname)
> pr_err("only give one device per ARRAY line: %s and %s\n",
> diff --git a/mdopen.c b/mdopen.c
> index c4f1c12..0f3a244 100644
> --- a/mdopen.c
> +++ b/mdopen.c
> @@ -174,19 +174,25 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> num = strtoul(e, NULL, 10);
> strcpy(cname, dev+5);
> cname[e-(dev+5)] = 0;
> - /* name *must* be mdXX or md_dXX in this context */
> + /* name is not mdXX or md_dXX */
> if (num < 0 ||
> (strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
> - pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
> - dev, dev+5);
> - return -1;
> + if (strncmp(cname, "md_", 3) != 0) {
> + pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
> + dev, dev+5);
> + return -1;
> + }
> + /* /dev/md_<string> format */
> + strcpy(cname, dev+8);
> + num = -1;
> + } else {
> + if (strcmp(cname, "md") == 0)
> + use_mdp = 0;
> + else
> + use_mdp = 1;
> + /* recreate name: /dev/md/0 or /dev/md/d0 */
> + sprintf(cname, "%s%d", use_mdp?"d":"", num);
> }
> - if (strcmp(cname, "md") == 0)
> - use_mdp = 0;
> - else
> - use_mdp = 1;
> - /* recreate name: /dev/md/0 or /dev/md/d0 */
> - sprintf(cname, "%s%d", use_mdp?"d":"", num);
> } else
> strcpy(cname, dev);
>
>
^ permalink raw reply
* [PATCH] imsm: rebuild from 2-disk RAID10
From: Tomasz Majchrzak @ 2017-08-10 13:47 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Tomasz Majchrzak
When RAID10 loses 2 disks and it is still operational, it cannot be
rebuilt. The rebuild process starts for the first disk and completes,
however completion is not recorded in metadata. There is an assumption
that rebuild completion corresponds to transition from degraded to
normal state. It's not the case for 2-disk RAID10 as it's still degraded
after rebuild to first disk completes.
Check if disk rebuild flag is set in the second map and clear it. So far it
has been checked only in the first map (where it was not set). The flag in
the second map has not been cleared but rebuild completion dropped second
map so the problem was not visible.
If rebuild completion is notified and array still has failed disks and is in
degraded state, check first if rebuild position is really unset (the same
check as for array in normal state). If so, mark migration as done but don't
change array state (it should remain degraded). Update failed disk number.
On rebuild start don't clear the rebuild flag in the destination map for all
the drives because failed state is lost for one of them. Just do a copy of
a map and clear the flag in the destination map for the disk that goes into
rebuild. Similarily preserve the rebuild flag in the map during disk removal.
If the disk is missing on array start and migration has been in progress,
don't just cancel it. Check first if maybe one of the disks was not under
rebuild (rebuild flag present both in source and destination map). If so,
rebuild was running despite of failed disk so there is no need to cancel
migration.
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
super-intel.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 67 insertions(+), 10 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 51b7cc3..125c3a9 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4023,7 +4023,7 @@ static void migrate(struct imsm_dev *dev, struct intel_super *super,
/* duplicate and then set the target end state in map[0] */
memcpy(dest, src, sizeof_imsm_map(src));
- if (migr_type == MIGR_REBUILD || migr_type == MIGR_GEN_MIGR) {
+ if (migr_type == MIGR_GEN_MIGR) {
__u32 ord;
int i;
@@ -7936,14 +7936,35 @@ static void handle_missing(struct intel_super *super, struct imsm_dev *dev)
/* end process for initialization and rebuild only
*/
if (is_gen_migration(dev) == 0) {
- __u8 map_state;
- int failed;
+ int failed = imsm_count_failed(super, dev, MAP_0);
- failed = imsm_count_failed(super, dev, MAP_0);
- map_state = imsm_check_degraded(super, dev, failed, MAP_0);
+ if (failed) {
+ __u8 map_state;
+ struct imsm_map *map = get_imsm_map(dev, MAP_0);
+ struct imsm_map *map1;
+ int i, ord, ord_map1;
+ int rebuilt = 1;
- if (failed)
- end_migration(dev, super, map_state);
+ for (i = 0; i < map->num_members; i++) {
+ ord = get_imsm_ord_tbl_ent(dev, i, MAP_0);
+ if (!(ord & IMSM_ORD_REBUILD))
+ continue;
+
+ map1 = get_imsm_map(dev, MAP_1);
+ if (!map1)
+ continue;
+
+ ord_map1 = __le32_to_cpu(map1->disk_ord_tbl[i]);
+ if (ord_map1 & IMSM_ORD_REBUILD)
+ rebuilt = 0;
+ }
+
+ if (rebuilt) {
+ map_state = imsm_check_degraded(super, dev,
+ failed, MAP_0);
+ end_migration(dev, super, map_state);
+ }
+ }
}
for (dl = super->missing; dl; dl = dl->next)
mark_missing(super, dev, &dl->disk, dl->index);
@@ -8225,8 +8246,10 @@ static void imsm_set_disk(struct active_array *a, int n, int state)
int failed;
int ord;
__u8 map_state;
+ int rebuild_done = 0;
+ int i;
- ord = imsm_disk_slot_to_ord(a, n);
+ ord = get_imsm_ord_tbl_ent(dev, n, MAP_X);
if (ord < 0)
return;
@@ -8244,6 +8267,7 @@ static void imsm_set_disk(struct active_array *a, int n, int state)
struct imsm_map *migr_map = get_imsm_map(dev, MAP_1);
set_imsm_ord_tbl_ent(migr_map, n, ord_to_idx(ord));
+ rebuild_done = 1;
super->updates_pending++;
}
@@ -8306,7 +8330,39 @@ static void imsm_set_disk(struct active_array *a, int n, int state)
dprintf_cont(" Map state change");
end_migration(dev, super, map_state);
super->updates_pending++;
+ } else if (!rebuild_done) {
+ break;
+ }
+
+ /* check if recovery is really finished */
+ for (mdi = a->info.devs; mdi ; mdi = mdi->next)
+ if (mdi->recovery_start != MaxSector) {
+ recovery_not_finished = 1;
+ break;
+ }
+ if (recovery_not_finished) {
+ dprintf_cont("\n");
+ dprintf("Rebuild has not finished yet, state not changed");
+ if (a->last_checkpoint < mdi->recovery_start) {
+ a->last_checkpoint =
+ mdi->recovery_start;
+ super->updates_pending++;
+ }
+ break;
}
+
+ dprintf_cont(" Rebuild done, still degraded");
+ dev->vol.migr_state = 0;
+ set_migr_type(dev, 0);
+ dev->vol.curr_migr_unit = 0;
+
+ for (i = 0; i < map->num_members; i++) {
+ int idx = get_imsm_ord_tbl_ent(dev, i, MAP_0);
+
+ if (idx & IMSM_ORD_REBUILD)
+ map->failed_disk_num = i;
+ }
+ super->updates_pending++;
break;
}
if (is_gen_migration(dev)) {
@@ -9936,7 +9992,7 @@ static void imsm_delete(struct intel_super *super, struct dl **dlp, unsigned ind
struct imsm_dev *dev;
struct imsm_map *map;
unsigned int i, j, num_members;
- __u32 ord;
+ __u32 ord, ord_map0;
struct bbm_log *log = super->bbm_log;
dprintf("deleting device[%d] from imsm_super\n", index);
@@ -9958,12 +10014,13 @@ static void imsm_delete(struct intel_super *super, struct dl **dlp, unsigned ind
* ord-flags to the first map
*/
ord = get_imsm_ord_tbl_ent(dev, j, MAP_X);
+ ord_map0 = get_imsm_ord_tbl_ent(dev, j, MAP_0);
if (ord_to_idx(ord) <= index)
continue;
map = get_imsm_map(dev, MAP_0);
- set_imsm_ord_tbl_ent(map, j, ord_to_idx(ord - 1));
+ set_imsm_ord_tbl_ent(map, j, ord_map0 - 1);
map = get_imsm_map(dev, MAP_1);
if (map)
set_imsm_ord_tbl_ent(map, j, ord - 1);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 14:14 UTC (permalink / raw)
To: Goldwyn Rodrigues, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <f8d2c0e3-e81f-55e7-2645-4598af34b367@suse.de>
On 08/10/2017 05:38 AM, Goldwyn Rodrigues wrote:
>
>
> On 08/09/2017 09:18 PM, Jens Axboe wrote:
>> On 08/08/2017 02:36 PM, Jens Axboe wrote:
>>> On 08/08/2017 02:32 PM, Shaohua Li wrote:
>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>>> --- a/include/linux/blkdev.h
>>>>> +++ b/include/linux/blkdev.h
>>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>>
>>> Does this work on 32-bit, where sizeof(unsigned long) == 32?
>>
>> I didn't get an answer to this one.
>>
>
> Oh, I assumed the question is rhetorical.
> No, it will not work on 32-bit. I was planning to change the field
> queue_flags to u64. Is that okay?
No, besides that would not work with set/test_bit() and friends. Grab
a free bit instead.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 14:23 UTC (permalink / raw)
To: Goldwyn Rodrigues, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel
In-Reply-To: <bfbd8644-1cd9-a748-1039-3dc08e4c45f5@suse.de>
On 08/10/2017 05:49 AM, Goldwyn Rodrigues wrote:
>
>
> On 08/09/2017 09:17 PM, Jens Axboe wrote:
>> On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>>>> I shall check again. Where do you see this happening?
>>>>>>>>
>>>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>>>> Please see blk_queue_split.
>>>>>>>>
>>>>>>>
>>>>>>> In that case, the bio end_io function is chained and the bio of
>>>>>>> the split will replicate the error to the parent (if not already
>>>>>>> set).
>>>>>>
>>>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>>>> of the bio probably already dispatched to disk (if the bio is
>>>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>>>> block and dispatch to disk), what will application be going to do?
>>>>>> I think this is different to other IO errors. FOr other IO errors,
>>>>>> application will handle the error, while we ask app to retry the
>>>>>> whole bio here and app doesn't know part of bio is already written
>>>>>> to disk.
>>>>>
>>>>> It is the same as for other I/O errors as well, such as EIO. You do
>>>>> not know which bio of all submitted bio's returned the error EIO.
>>>>> The application would and should consider the whole I/O as failed.
>>>>>
>>>>> The user application does not know of bios, or how it is going to be
>>>>> split in the underlying layers. It knows at the system call level.
>>>>> In this case, the EAGAIN will be returned to the user for the whole
>>>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>>>> out using dio->io_error. You can read about it at the patch header
>>>>> for the initial patchset at [1].
>>>>>
>>>>> Use case: It is for applications having two threads, a compute
>>>>> thread and an I/O thread. It would try to push AIO as much as
>>>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>>>> would pass it on to I/O thread which would perform without
>>>>> RWF_NOWAIT. End result if done right is you save on context switches
>>>>> and all the synchronization/messaging machinery to perform I/O.
>>>>>
>>>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>>>
>>>> Yes, I knew the concept, but I didn't see previous patches mentioned
>>>> the -EAGAIN actually should be taken as a real IO error. This means a
>>>> lot to applications and make the API hard to use. I'm wondering if we
>>>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>>>> only mean 'try again'.
>>>
>>> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
>>> the API is hard to use? Do you have a case to back it up?
>>
>> Because it is hard to use, and potentially suboptimal. Let's say you're
>> doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
>> short write, or do we return EWOULDBLOCK? If the latter, then that
>> really sucks from an API point of view.
>>
>>> No, not splitting the bio does not make sense here. I do not see any
>>> advantage in it, unless you can present a case otherwise.
>>
>> It ties back into the "hard to use" that I do agree with IFF we don't
>> return the short write. It's hard for an application to use that
>> efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
>> full 1MB from a different context.
>>
>
> It returns the error code only and not short reads/writes. But isn't
> that true for all system calls in case of error?
It's not a hard error. If you wrote 896K in the example above, I'd
really expect the return value to be 896*1024. The API is hard to use
efficiently, if that's not the case.
> For aio, there are two result fields in io_event out of which one could
> be used for error while the other be used for amount of writes/reads
> performed. However, only one is used. This will not work with
> pread()/pwrite() calls though because of the limitation of return values.
Don't invent something new for this, the mechanism already exists for
returning a short read or write. That's how all of them have worked for
decades.
> Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
> offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
> successful. What short return value should the system call return?
It should return 128*1024, since that's how much was successfully done
from the start offset. But yes, this is exactly the point that I brought
up, and why contesting Shaohua's suggestion to perhaps treat splits
differently should not be discarded so quickly.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jan Kara @ 2017-08-10 14:25 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: Jens Axboe, Shaohua Li, linux-block, hch, jack, linux-raid,
dm-devel
In-Reply-To: <bfbd8644-1cd9-a748-1039-3dc08e4c45f5@suse.de>
On Thu 10-08-17 06:49:53, Goldwyn Rodrigues wrote:
> On 08/09/2017 09:17 PM, Jens Axboe wrote:
> > On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
> >>>>>>>> No, from a multi-device point of view, this is inconsistent. I
> >>>>>>>> have tried the request bio returns -EAGAIN before the split, but
> >>>>>>>> I shall check again. Where do you see this happening?
> >>>>>>>
> >>>>>>> No, this isn't multi-device specific, any driver can do it.
> >>>>>>> Please see blk_queue_split.
> >>>>>>>
> >>>>>>
> >>>>>> In that case, the bio end_io function is chained and the bio of
> >>>>>> the split will replicate the error to the parent (if not already
> >>>>>> set).
> >>>>>
> >>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
> >>>>> of the bio probably already dispatched to disk (if the bio is
> >>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
> >>>>> block and dispatch to disk), what will application be going to do?
> >>>>> I think this is different to other IO errors. FOr other IO errors,
> >>>>> application will handle the error, while we ask app to retry the
> >>>>> whole bio here and app doesn't know part of bio is already written
> >>>>> to disk.
> >>>>
> >>>> It is the same as for other I/O errors as well, such as EIO. You do
> >>>> not know which bio of all submitted bio's returned the error EIO.
> >>>> The application would and should consider the whole I/O as failed.
> >>>>
> >>>> The user application does not know of bios, or how it is going to be
> >>>> split in the underlying layers. It knows at the system call level.
> >>>> In this case, the EAGAIN will be returned to the user for the whole
> >>>> I/O not as a part of the I/O. It is up to application to try the I/O
> >>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
> >>>> out using dio->io_error. You can read about it at the patch header
> >>>> for the initial patchset at [1].
> >>>>
> >>>> Use case: It is for applications having two threads, a compute
> >>>> thread and an I/O thread. It would try to push AIO as much as
> >>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
> >>>> would pass it on to I/O thread which would perform without
> >>>> RWF_NOWAIT. End result if done right is you save on context switches
> >>>> and all the synchronization/messaging machinery to perform I/O.
> >>>>
> >>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
> >>>
> >>> Yes, I knew the concept, but I didn't see previous patches mentioned
> >>> the -EAGAIN actually should be taken as a real IO error. This means a
> >>> lot to applications and make the API hard to use. I'm wondering if we
> >>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
> >>> only mean 'try again'.
> >>
> >> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
> >> the API is hard to use? Do you have a case to back it up?
> >
> > Because it is hard to use, and potentially suboptimal. Let's say you're
> > doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
> > short write, or do we return EWOULDBLOCK? If the latter, then that
> > really sucks from an API point of view.
> >
> >> No, not splitting the bio does not make sense here. I do not see any
> >> advantage in it, unless you can present a case otherwise.
> >
> > It ties back into the "hard to use" that I do agree with IFF we don't
> > return the short write. It's hard for an application to use that
> > efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
> > full 1MB from a different context.
> >
>
> It returns the error code only and not short reads/writes. But isn't
> that true for all system calls in case of error?
>
> For aio, there are two result fields in io_event out of which one could
> be used for error while the other be used for amount of writes/reads
> performed. However, only one is used. This will not work with
> pread()/pwrite() calls though because of the limitation of return values.
>
> Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
> offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
> successful. What short return value should the system call return?
This is indeed tricky. If an application submits 1MB write, I don't think
we can afford to just write arbitrary subset of it. That just IMHO too much
violates how writes traditionally behaved. Even short writes trigger bugs
in various applications but I'm willing to require that applications using
NOWAIT IO can handle these. However writing arbitrary subset looks like a
nasty catch. IMHO we should not submit further bios until we are sure
current one does not return EWOULDBLOCK when splitting a larger one...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 14:28 UTC (permalink / raw)
To: Jan Kara, Goldwyn Rodrigues
Cc: Shaohua Li, linux-block, hch, linux-raid, dm-devel
In-Reply-To: <20170810142515.GC14925@quack2.suse.cz>
On 08/10/2017 08:25 AM, Jan Kara wrote:
> On Thu 10-08-17 06:49:53, Goldwyn Rodrigues wrote:
>> On 08/09/2017 09:17 PM, Jens Axboe wrote:
>>> On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>>>>> I shall check again. Where do you see this happening?
>>>>>>>>>
>>>>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>>>>> Please see blk_queue_split.
>>>>>>>>>
>>>>>>>>
>>>>>>>> In that case, the bio end_io function is chained and the bio of
>>>>>>>> the split will replicate the error to the parent (if not already
>>>>>>>> set).
>>>>>>>
>>>>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>>>>> of the bio probably already dispatched to disk (if the bio is
>>>>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>>>>> block and dispatch to disk), what will application be going to do?
>>>>>>> I think this is different to other IO errors. FOr other IO errors,
>>>>>>> application will handle the error, while we ask app to retry the
>>>>>>> whole bio here and app doesn't know part of bio is already written
>>>>>>> to disk.
>>>>>>
>>>>>> It is the same as for other I/O errors as well, such as EIO. You do
>>>>>> not know which bio of all submitted bio's returned the error EIO.
>>>>>> The application would and should consider the whole I/O as failed.
>>>>>>
>>>>>> The user application does not know of bios, or how it is going to be
>>>>>> split in the underlying layers. It knows at the system call level.
>>>>>> In this case, the EAGAIN will be returned to the user for the whole
>>>>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>>>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>>>>> out using dio->io_error. You can read about it at the patch header
>>>>>> for the initial patchset at [1].
>>>>>>
>>>>>> Use case: It is for applications having two threads, a compute
>>>>>> thread and an I/O thread. It would try to push AIO as much as
>>>>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>>>>> would pass it on to I/O thread which would perform without
>>>>>> RWF_NOWAIT. End result if done right is you save on context switches
>>>>>> and all the synchronization/messaging machinery to perform I/O.
>>>>>>
>>>>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>>>>
>>>>> Yes, I knew the concept, but I didn't see previous patches mentioned
>>>>> the -EAGAIN actually should be taken as a real IO error. This means a
>>>>> lot to applications and make the API hard to use. I'm wondering if we
>>>>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>>>>> only mean 'try again'.
>>>>
>>>> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
>>>> the API is hard to use? Do you have a case to back it up?
>>>
>>> Because it is hard to use, and potentially suboptimal. Let's say you're
>>> doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
>>> short write, or do we return EWOULDBLOCK? If the latter, then that
>>> really sucks from an API point of view.
>>>
>>>> No, not splitting the bio does not make sense here. I do not see any
>>>> advantage in it, unless you can present a case otherwise.
>>>
>>> It ties back into the "hard to use" that I do agree with IFF we don't
>>> return the short write. It's hard for an application to use that
>>> efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
>>> full 1MB from a different context.
>>>
>>
>> It returns the error code only and not short reads/writes. But isn't
>> that true for all system calls in case of error?
>>
>> For aio, there are two result fields in io_event out of which one could
>> be used for error while the other be used for amount of writes/reads
>> performed. However, only one is used. This will not work with
>> pread()/pwrite() calls though because of the limitation of return values.
>>
>> Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
>> offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
>> successful. What short return value should the system call return?
>
> This is indeed tricky. If an application submits 1MB write, I don't think
> we can afford to just write arbitrary subset of it. That just IMHO too much
> violates how writes traditionally behaved. Even short writes trigger bugs
> in various applications but I'm willing to require that applications using
> NOWAIT IO can handle these. However writing arbitrary subset looks like a
> nasty catch. IMHO we should not submit further bios until we are sure
> current one does not return EWOULDBLOCK when splitting a larger one...
Exactly, that's the point that both Shaohua and I was getting at. Short
writes should be fine, especially if NOWAIT is set. Discontig writes
should also be OK, but it's horrible and inefficient. If we do that,
then using this feature is a net-loss, not a win by any stretch.
--
Jens Axboe
^ permalink raw reply
* migrate from mdadm to HW RAID
From: David Zampirolo @ 2017-08-10 15:24 UTC (permalink / raw)
To: linux-raid
Hi people,
I'm trying to move data from 2 software raid volumes /dev/md0 (/) and
/dev/md1 (swap)
made by mdadm to a phisical RAID over a server.
md0 and md1 are both raid level 1 composed by 2 partitions over sda and sdb
Anyone have already done that?
The problem is that i need to work only on the disk-image i cannot
stop or modify the original workstation.
any suggestion?
Thanks for your time
David
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-10 17:15 UTC (permalink / raw)
To: Jens Axboe, Jan Kara; +Cc: Shaohua Li, linux-block, hch, linux-raid, dm-devel
In-Reply-To: <e0c72d10-388e-efac-242d-1b3542490085@kernel.dk>
On 08/10/2017 09:28 AM, Jens Axboe wrote:
> On 08/10/2017 08:25 AM, Jan Kara wrote:
>> On Thu 10-08-17 06:49:53, Goldwyn Rodrigues wrote:
>>> On 08/09/2017 09:17 PM, Jens Axboe wrote:
>>>> On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>>>>>> I shall check again. Where do you see this happening?
>>>>>>>>>>
>>>>>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>>>>>> Please see blk_queue_split.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> In that case, the bio end_io function is chained and the bio of
>>>>>>>>> the split will replicate the error to the parent (if not already
>>>>>>>>> set).
>>>>>>>>
>>>>>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>>>>>> of the bio probably already dispatched to disk (if the bio is
>>>>>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>>>>>> block and dispatch to disk), what will application be going to do?
>>>>>>>> I think this is different to other IO errors. FOr other IO errors,
>>>>>>>> application will handle the error, while we ask app to retry the
>>>>>>>> whole bio here and app doesn't know part of bio is already written
>>>>>>>> to disk.
>>>>>>>
>>>>>>> It is the same as for other I/O errors as well, such as EIO. You do
>>>>>>> not know which bio of all submitted bio's returned the error EIO.
>>>>>>> The application would and should consider the whole I/O as failed.
>>>>>>>
>>>>>>> The user application does not know of bios, or how it is going to be
>>>>>>> split in the underlying layers. It knows at the system call level.
>>>>>>> In this case, the EAGAIN will be returned to the user for the whole
>>>>>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>>>>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>>>>>> out using dio->io_error. You can read about it at the patch header
>>>>>>> for the initial patchset at [1].
>>>>>>>
>>>>>>> Use case: It is for applications having two threads, a compute
>>>>>>> thread and an I/O thread. It would try to push AIO as much as
>>>>>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>>>>>> would pass it on to I/O thread which would perform without
>>>>>>> RWF_NOWAIT. End result if done right is you save on context switches
>>>>>>> and all the synchronization/messaging machinery to perform I/O.
>>>>>>>
>>>>>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>>>>>
>>>>>> Yes, I knew the concept, but I didn't see previous patches mentioned
>>>>>> the -EAGAIN actually should be taken as a real IO error. This means a
>>>>>> lot to applications and make the API hard to use. I'm wondering if we
>>>>>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>>>>>> only mean 'try again'.
>>>>>
>>>>> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
>>>>> the API is hard to use? Do you have a case to back it up?
>>>>
>>>> Because it is hard to use, and potentially suboptimal. Let's say you're
>>>> doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
>>>> short write, or do we return EWOULDBLOCK? If the latter, then that
>>>> really sucks from an API point of view.
>>>>
>>>>> No, not splitting the bio does not make sense here. I do not see any
>>>>> advantage in it, unless you can present a case otherwise.
>>>>
>>>> It ties back into the "hard to use" that I do agree with IFF we don't
>>>> return the short write. It's hard for an application to use that
>>>> efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
>>>> full 1MB from a different context.
>>>>
>>>
>>> It returns the error code only and not short reads/writes. But isn't
>>> that true for all system calls in case of error?
>>>
>>> For aio, there are two result fields in io_event out of which one could
>>> be used for error while the other be used for amount of writes/reads
>>> performed. However, only one is used. This will not work with
>>> pread()/pwrite() calls though because of the limitation of return values.
>>>
>>> Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
>>> offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
>>> successful. What short return value should the system call return?
>>
>> This is indeed tricky. If an application submits 1MB write, I don't think
>> we can afford to just write arbitrary subset of it. That just IMHO too much
>> violates how writes traditionally behaved. Even short writes trigger bugs
>> in various applications but I'm willing to require that applications using
>> NOWAIT IO can handle these. However writing arbitrary subset looks like a
>> nasty catch. IMHO we should not submit further bios until we are sure
>> current one does not return EWOULDBLOCK when splitting a larger one...
>
> Exactly, that's the point that both Shaohua and I was getting at. Short
> writes should be fine, especially if NOWAIT is set. Discontig writes
> should also be OK, but it's horrible and inefficient. If we do that,
> then using this feature is a net-loss, not a win by any stretch.
>
To make sure I understand this, we disable bio splits for NOWAIT bio so
we return EWOULDBLOCK for the entire I/O.
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Goldwyn Rodrigues @ 2017-08-10 17:15 UTC (permalink / raw)
To: Jens Axboe, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <0714ec15-9dad-f7a3-4f09-e7624f109de6@kernel.dk>
On 08/10/2017 09:14 AM, Jens Axboe wrote:
> On 08/10/2017 05:38 AM, Goldwyn Rodrigues wrote:
>>
>>
>> On 08/09/2017 09:18 PM, Jens Axboe wrote:
>>> On 08/08/2017 02:36 PM, Jens Axboe wrote:
>>>> On 08/08/2017 02:32 PM, Shaohua Li wrote:
>>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>>>> --- a/include/linux/blkdev.h
>>>>>> +++ b/include/linux/blkdev.h
>>>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>>>
>>>> Does this work on 32-bit, where sizeof(unsigned long) == 32?
>>>
>>> I didn't get an answer to this one.
>>>
>>
>> Oh, I assumed the question is rhetorical.
>> No, it will not work on 32-bit. I was planning to change the field
>> queue_flags to u64. Is that okay?
>
> No, besides that would not work with set/test_bit() and friends. Grab
> a free bit instead.
>
Which bit is free? I don't see any gaps in QUEUE_FLAG_*, and I am not
sure if any one is unused.
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 17:17 UTC (permalink / raw)
To: Goldwyn Rodrigues, Shaohua Li
Cc: linux-block, hch, jack, linux-raid, dm-devel, Goldwyn Rodrigues
In-Reply-To: <a395c6fc-3aea-53e5-6cf6-c2a01acd70b1@suse.de>
On 08/10/2017 11:15 AM, Goldwyn Rodrigues wrote:
>
>
> On 08/10/2017 09:14 AM, Jens Axboe wrote:
>> On 08/10/2017 05:38 AM, Goldwyn Rodrigues wrote:
>>>
>>>
>>> On 08/09/2017 09:18 PM, Jens Axboe wrote:
>>>> On 08/08/2017 02:36 PM, Jens Axboe wrote:
>>>>> On 08/08/2017 02:32 PM, Shaohua Li wrote:
>>>>>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>>>>>> index 25f6a0cb27d3..fae021ebec1b 100644
>>>>>>> --- a/include/linux/blkdev.h
>>>>>>> +++ b/include/linux/blkdev.h
>>>>>>> @@ -633,6 +633,7 @@ struct request_queue {
>>>>>>> #define QUEUE_FLAG_REGISTERED 29 /* queue has been registered to a disk */
>>>>>>> #define QUEUE_FLAG_SCSI_PASSTHROUGH 30 /* queue supports SCSI commands */
>>>>>>> #define QUEUE_FLAG_QUIESCED 31 /* queue has been quiesced */
>>>>>>> +#define QUEUE_FLAG_NOWAIT 32 /* stack device driver supports REQ_NOWAIT */
>>>>>
>>>>> Does this work on 32-bit, where sizeof(unsigned long) == 32?
>>>>
>>>> I didn't get an answer to this one.
>>>>
>>>
>>> Oh, I assumed the question is rhetorical.
>>> No, it will not work on 32-bit. I was planning to change the field
>>> queue_flags to u64. Is that okay?
>>
>> No, besides that would not work with set/test_bit() and friends. Grab
>> a free bit instead.
>>
>
> Which bit is free? I don't see any gaps in QUEUE_FLAG_*, and I am not
> sure if any one is unused.
Bit 0 is free in mainline, and I just looked, and two other bits are
gone as well:
http://git.kernel.dk/cgit/linux-block/commit/?h=for-4.14/block&id=e743eb1ecd5564b5ae0a4a76c1566f748a358839
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 1/9] QUEUE_FLAG_NOWAIT to indicate device supports nowait
From: Jens Axboe @ 2017-08-10 17:20 UTC (permalink / raw)
To: Goldwyn Rodrigues, Jan Kara
Cc: Shaohua Li, linux-block, hch, linux-raid, dm-devel
In-Reply-To: <05106658-7bc4-4054-fbda-3b33ea32e215@suse.de>
On 08/10/2017 11:15 AM, Goldwyn Rodrigues wrote:
>
>
> On 08/10/2017 09:28 AM, Jens Axboe wrote:
>> On 08/10/2017 08:25 AM, Jan Kara wrote:
>>> On Thu 10-08-17 06:49:53, Goldwyn Rodrigues wrote:
>>>> On 08/09/2017 09:17 PM, Jens Axboe wrote:
>>>>> On 08/09/2017 08:07 PM, Goldwyn Rodrigues wrote:
>>>>>>>>>>>> No, from a multi-device point of view, this is inconsistent. I
>>>>>>>>>>>> have tried the request bio returns -EAGAIN before the split, but
>>>>>>>>>>>> I shall check again. Where do you see this happening?
>>>>>>>>>>>
>>>>>>>>>>> No, this isn't multi-device specific, any driver can do it.
>>>>>>>>>>> Please see blk_queue_split.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> In that case, the bio end_io function is chained and the bio of
>>>>>>>>>> the split will replicate the error to the parent (if not already
>>>>>>>>>> set).
>>>>>>>>>
>>>>>>>>> this doesn't answer my question. So if a bio returns -EAGAIN, part
>>>>>>>>> of the bio probably already dispatched to disk (if the bio is
>>>>>>>>> splitted to 2 bios, one returns -EAGAIN, the other one doesn't
>>>>>>>>> block and dispatch to disk), what will application be going to do?
>>>>>>>>> I think this is different to other IO errors. FOr other IO errors,
>>>>>>>>> application will handle the error, while we ask app to retry the
>>>>>>>>> whole bio here and app doesn't know part of bio is already written
>>>>>>>>> to disk.
>>>>>>>>
>>>>>>>> It is the same as for other I/O errors as well, such as EIO. You do
>>>>>>>> not know which bio of all submitted bio's returned the error EIO.
>>>>>>>> The application would and should consider the whole I/O as failed.
>>>>>>>>
>>>>>>>> The user application does not know of bios, or how it is going to be
>>>>>>>> split in the underlying layers. It knows at the system call level.
>>>>>>>> In this case, the EAGAIN will be returned to the user for the whole
>>>>>>>> I/O not as a part of the I/O. It is up to application to try the I/O
>>>>>>>> again with or without RWF_NOWAIT set. In direct I/O, it is bubbled
>>>>>>>> out using dio->io_error. You can read about it at the patch header
>>>>>>>> for the initial patchset at [1].
>>>>>>>>
>>>>>>>> Use case: It is for applications having two threads, a compute
>>>>>>>> thread and an I/O thread. It would try to push AIO as much as
>>>>>>>> possible in the compute thread using RWF_NOWAIT, and if it fails,
>>>>>>>> would pass it on to I/O thread which would perform without
>>>>>>>> RWF_NOWAIT. End result if done right is you save on context switches
>>>>>>>> and all the synchronization/messaging machinery to perform I/O.
>>>>>>>>
>>>>>>>> [1] http://marc.info/?l=linux-block&m=149789003305876&w=2
>>>>>>>
>>>>>>> Yes, I knew the concept, but I didn't see previous patches mentioned
>>>>>>> the -EAGAIN actually should be taken as a real IO error. This means a
>>>>>>> lot to applications and make the API hard to use. I'm wondering if we
>>>>>>> should disable bio split for NOWAIT bio, which will make the -EAGAIN
>>>>>>> only mean 'try again'.
>>>>>>
>>>>>> Don't take it as EAGAIN, but read it as EWOULDBLOCK. Why do you say
>>>>>> the API is hard to use? Do you have a case to back it up?
>>>>>
>>>>> Because it is hard to use, and potentially suboptimal. Let's say you're
>>>>> doing a 1MB write, we hit EWOULDBLOCK for the last split. Do we return a
>>>>> short write, or do we return EWOULDBLOCK? If the latter, then that
>>>>> really sucks from an API point of view.
>>>>>
>>>>>> No, not splitting the bio does not make sense here. I do not see any
>>>>>> advantage in it, unless you can present a case otherwise.
>>>>>
>>>>> It ties back into the "hard to use" that I do agree with IFF we don't
>>>>> return the short write. It's hard for an application to use that
>>>>> efficiently, if we write 1MB-128K but get EWOULDBLOCK, the re-write the
>>>>> full 1MB from a different context.
>>>>>
>>>>
>>>> It returns the error code only and not short reads/writes. But isn't
>>>> that true for all system calls in case of error?
>>>>
>>>> For aio, there are two result fields in io_event out of which one could
>>>> be used for error while the other be used for amount of writes/reads
>>>> performed. However, only one is used. This will not work with
>>>> pread()/pwrite() calls though because of the limitation of return values.
>>>>
>>>> Finally, what if the EWOULDBLOCK is returned for an earlier bio (say
>>>> offset 128k) for a 1MB pwrite(), while the rest of the 7 128K are
>>>> successful. What short return value should the system call return?
>>>
>>> This is indeed tricky. If an application submits 1MB write, I don't think
>>> we can afford to just write arbitrary subset of it. That just IMHO too much
>>> violates how writes traditionally behaved. Even short writes trigger bugs
>>> in various applications but I'm willing to require that applications using
>>> NOWAIT IO can handle these. However writing arbitrary subset looks like a
>>> nasty catch. IMHO we should not submit further bios until we are sure
>>> current one does not return EWOULDBLOCK when splitting a larger one...
>>
>> Exactly, that's the point that both Shaohua and I was getting at. Short
>> writes should be fine, especially if NOWAIT is set. Discontig writes
>> should also be OK, but it's horrible and inefficient. If we do that,
>> then using this feature is a net-loss, not a win by any stretch.
>>
>
> To make sure I understand this, we disable bio splits for NOWAIT bio so
> we return EWOULDBLOCK for the entire I/O.
That's also not great, since splits is a common operation, and the majority
of splits can proceed without hitting out-of-resources. So ideally we'd
handle that case, but in a saner fashion than the laissez faire approach
that the current patchset takes.
--
Jens Axboe
^ permalink raw reply
* md0 device gone, disks busy
From: Mr Typo @ 2017-08-10 20:26 UTC (permalink / raw)
To: linux-raid
Hello everyone,
i had a power outage 2 days ago and when i came home i found my linux
box not booting up anymore. After some commands i found out that there
is a problem with a device not beeing mounted because md0 is missing.
I tried to find out what is going on, but i cannot understand what is
going on. i hope you can help me. Linux is a CentOS7.
I am posting the current status of the box below:
thank you all in advance
kind regards
Andy
[root@quad ~]# mdadm -V
mdadm - v3.4 - 28th January 2016
[root@quad ~]# uname -a
Linux quad 3.10.0-514.26.2.el7.x86_64 #1 SMP Tue Jul 4 15:04:05 UTC
2017 x86_64 x86_64 x86_64 GNU/Linux
[root@quad ~]# cat /proc/mdstat
Personalities :
unused devices: <none>
[root@quad ~]# ls -la /dev/md0
ls: cannot access /dev/md0: No such file or directory
[root@quad ~]# mdadm -Esv
ARRAY /dev/md/0 level=raid5 metadata=1.2 num-devices=4
UUID=9d020f27:c0542472:b95a18d2:5741114d name=quad.:0
devices=/dev/sda1,/dev/sdb1,/dev/sdc1,/dev/sdd1
[root@quad ~]# mdadm -A -v /dev/md0
mdadm: looking for devices for /dev/md0
mdadm: /dev/sda1 is busy - skipping
mdadm: /dev/sdb1 is busy - skipping
mdadm: /dev/sdc1 is busy - skipping
mdadm: /dev/sdd1 is busy - skipping
[root@quad ~]# cat /proc/partitions |grep -e sd[a-d]
8 32 1953514584 sdc
8 33 1953513560 sdc1
8 0 1953514584 sda
8 1 1953513560 sda1
8 16 1953514584 sdb
8 17 1953513560 sdb1
8 48 1953514584 sdd
8 49 1953513560 sdd1
[root@quad ~]# cat /etc/mdadm.conf
DEVICE /dev/sda1 /dev/sdb1 /dev/sdc1 /dev/sdd1
ARRAY /dev/md0 metadata=1.2 name=quad:0 UUID=9d020f27:c0542472:b95a18d2:5741114d
[root@quad ~]# mdadm --examine --scan
ARRAY /dev/md/0 metadata=1.2 UUID=9d020f27:c0542472:b95a18d2:5741114d
name=quad:0
[root@quad ~]# cat /etc/mdadm/mdadm.conf
ARRAY /dev/md/0 metadata=1.2 UUID=9d020f27:c0542472:b95a18d2:5741114d
name=quad:
[root@quad ~]# lsof /dev/sda*
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
multipath 607 root 10r BLK 8,0 0t0 14507 /dev/sda
[root@quad ~]# mdadm --examine /dev/sd[a-d]*
/dev/sda:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sda1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 9d020f27:c0542472:b95a18d2:5741114d
Name : quad:0 (local to host quad)
Creation Time : Wed Aug 20 19:28:52 2014
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3906764976 (1862.89 GiB 2000.26 GB)
Array Size : 5860146816 (5588.67 GiB 6000.79 GB)
Used Dev Size : 3906764544 (1862.89 GiB 2000.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=432 sectors
State : clean
Device UUID : cee082b6:e5128634:bbaea65e:e75a2352
Update Time : Wed Aug 9 22:09:11 2017
Checksum : 9b6dc518 - correct
Events : 25456
Layout : left-symmetric
Chunk Size : 128K
Device Role : Active device 0
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
/dev/sdb:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdb1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 9d020f27:c0542472:b95a18d2:5741114d
Name : quad:0 (local to host quad)
Creation Time : Wed Aug 20 19:28:52 2014
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3906764976 (1862.89 GiB 2000.26 GB)
Array Size : 5860146816 (5588.67 GiB 6000.79 GB)
Used Dev Size : 3906764544 (1862.89 GiB 2000.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=432 sectors
State : clean
Device UUID : b217daeb:fcebf9a3:8f8fd682:3ce535c6
Update Time : Wed Aug 9 22:09:11 2017
Checksum : d87b403e - correct
Events : 25456
Layout : left-symmetric
Chunk Size : 128K
Device Role : Active device 1
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
/dev/sdc:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdc1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 9d020f27:c0542472:b95a18d2:5741114d
Name : quad:0 (local to host quad)
Creation Time : Wed Aug 20 19:28:52 2014
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3906764976 (1862.89 GiB 2000.26 GB)
Array Size : 5860146816 (5588.67 GiB 6000.79 GB)
Used Dev Size : 3906764544 (1862.89 GiB 2000.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=432 sectors
State : clean
Device UUID : f1a520b9:0894d2aa:d822db73:8dfd006b
Update Time : Wed Aug 9 22:09:11 2017
Checksum : 426a2224 - correct
Events : 25456
Layout : left-symmetric
Chunk Size : 128K
Device Role : Active device 2
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
/dev/sdd:
MBR Magic : aa55
Partition[0] : 3907027120 sectors at 2048 (type fd)
/dev/sdd1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 9d020f27:c0542472:b95a18d2:5741114d
Name : quad:0 (local to host quad)
Creation Time : Wed Aug 20 19:28:52 2014
Raid Level : raid5
Raid Devices : 4
Avail Dev Size : 3906764976 (1862.89 GiB 2000.26 GB)
Array Size : 5860146816 (5588.67 GiB 6000.79 GB)
Used Dev Size : 3906764544 (1862.89 GiB 2000.26 GB)
Data Offset : 262144 sectors
Super Offset : 8 sectors
Unused Space : before=262064 sectors, after=432 sectors
State : clean
Device UUID : 42270e8a:deb45300:a3e5de7e:74345b99
Update Time : Wed Aug 9 22:09:11 2017
Checksum : a236c146 - correct
Events : 25456
Layout : left-symmetric
Chunk Size : 128K
Device Role : Active device 3
Array State : AAAA ('A' == active, '.' == missing, 'R' == replacing)
^ permalink raw reply
* Re: md0 device gone, disks busy
From: Sarah Newman @ 2017-08-11 2:55 UTC (permalink / raw)
To: Mr Typo; +Cc: linux-raid
In-Reply-To: <CAHnOR0OrjQD2=o4uSU4N3_W+UvHQzpcW=oYB0cmPU_kPQxzhcg@mail.gmail.com>
On 08/10/2017 01:26 PM, Mr Typo wrote:
> Hello everyone,
>
> i had a power outage 2 days ago and when i came home i found my linux
> box not booting up anymore. After some commands i found out that there
> is a problem with a device not beeing mounted because md0 is missing.
>
> I tried to find out what is going on, but i cannot understand what is
> going on. i hope you can help me. Linux is a CentOS7.
>
> I am posting the current status of the box below:
>
> thank you all in advance
>
> kind regards
> Andy
> [root@quad ~]# mdadm -Esv
> ARRAY /dev/md/0 level=raid5 metadata=1.2 num-devices=4
> UUID=9d020f27:c0542472:b95a18d2:5741114d name=quad.:0
> devices=/dev/sda1,/dev/sdb1,/dev/sdc1,/dev/sdd1
It's running, it's just called /dev/md/0 instead. You should be able to temporarily change the name with
mdadm --stop /dev/md/0
mdadm --assemble /dev/md0 /dev/sda1 /dev/sdb1 /dev/sdc1 /dev/sdd1
I think for centos 7, to change what happens on boot you can edit /etc/mdadm.conf to explicitly name the array and run 'dracut -f' to update the
initial ram disk that loads on boot before the actual root file system is mounted. 'man mdadm.conf' has an example of the mdadm.conf syntax.
--Sarah
^ permalink raw reply
* [PATCH v4] bcache: Don't reinvent the wheel but use existing llist API
From: Byungchul Park @ 2017-08-11 4:42 UTC (permalink / raw)
To: kent.overstreet, shli
Cc: linux-bcache, linux-raid, i, nborisov, linux-kernel, kernel-team
Although llist provides proper APIs, they are not used. Make them used.
Signed-off-by: Byungchul Park <byungchul.park@lge.com>
Acked-by: Coly Li <colyli@suse.de>
---
drivers/md/bcache/closure.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
index 864e673..7d5286b 100644
--- a/drivers/md/bcache/closure.c
+++ b/drivers/md/bcache/closure.c
@@ -70,21 +70,10 @@ void __closure_wake_up(struct closure_waitlist *wait_list)
list = llist_del_all(&wait_list->list);
/* We first reverse the list to preserve FIFO ordering and fairness */
-
- while (list) {
- struct llist_node *t = list;
- list = llist_next(list);
-
- t->next = reverse;
- reverse = t;
- }
+ reverse = llist_reverse_order(list);
/* Then do the wakeups */
-
- while (reverse) {
- cl = container_of(reverse, struct closure, list);
- reverse = llist_next(reverse);
-
+ llist_for_each_entry(cl, reverse, list) {
closure_set_waiting(cl, 0);
closure_sub(cl, CLOSURE_WAITING + 1);
}
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox