Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Mike Snitzer]

This mail needs to be saent to stable@vger.kernel.org (now cc'd).

Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9

Thanks,
Mike

On Mon, Jul 27 2020 at  9:40am -0400,
John Donnelly <John.P.donnelly@oracle.com> wrote:

> From: Mike Snitzer <snitzer@redhat.com>
> 
> Discontinue issuing writethrough write IO in series to the origin and
> then cache.
> 
> Use bio_clone_fast() to create a new origin clone bio that will be
> mapped to the origin device and then bio_chain() it to the bio that gets
> remapped to the cache device. The origin clone bio does _not_ have a
> copy of the per_bio_data -- as such check_if_tick_bio_needed() will not
> be called.
> 
> The cache bio (parent bio) will not complete until the origin bio has
> completed -- this fulfills bio_clone_fast()'s requirements as well as
> the requirement to not complete the original IO until the write IO has
> completed to both the origin and cache device.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> 
> (cherry picked from commit 2df3bae9a6543e90042291707b8db0cbfbae9ee9)
> 
> Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record:
> save/restore bi_end_io and bi_integrity, version 4.14.188)
> 
> 70555970 introduced a mkfs.ext4 hang on a LVM device that has been
> modified with lvconvert --cachemode=writethrough.
> 
> Signed-off-by: John Donnelly <john.p.donnelly@oracle.com>
> Tested-by: John Donnelly <john.p.donnelly@oracle.com>
> Reviewed-by: Somasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>
> 
> conflict: drivers/md/dm-cache-target.c - Corrected syntax of
> writethrough_mode(&cache->feature) that was caught by
> arm compiler.
> 
> cc: stable@vger.kernel.org
> cc: snitzer@redhat.com
> ---
> drivers/md/dm-cache-target.c | 54 ++++++++++++++++++++++++------------
> 1 file changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 69cdb29ef6be..8241b7c36655 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -450,6 +450,7 @@ struct cache {
> struct work_struct migration_worker;
> struct delayed_work waker;
> struct dm_bio_prison_v2 *prison;
> + struct bio_set *bs;
> mempool_t *migration_pool;
> @@ -868,16 +869,23 @@ static void check_if_tick_bio_needed(struct
> cache *cache, struct bio *bio)
> spin_unlock_irqrestore(&cache->lock, flags);
> }
> -static void remap_to_origin_clear_discard(struct cache *cache,
> struct bio *bio,
> - dm_oblock_t oblock)
> +static void __remap_to_origin_clear_discard(struct cache *cache,
> struct bio *bio,
> + dm_oblock_t oblock, bool bio_has_pbd)
> {
> - // FIXME: this is called way too much.
> - check_if_tick_bio_needed(cache, bio);
> + if (bio_has_pbd)
> + check_if_tick_bio_needed(cache, bio);
> remap_to_origin(cache, bio);
> if (bio_data_dir(bio) == WRITE)
> clear_discard(cache, oblock_to_dblock(cache, oblock));
> }
> +static void remap_to_origin_clear_discard(struct cache *cache,
> struct bio *bio,
> + dm_oblock_t oblock)
> +{
> + // FIXME: check_if_tick_bio_needed() is called way too much
> through this interface
> + __remap_to_origin_clear_discard(cache, bio, oblock, true);
> +}
> +
> static void remap_to_cache_dirty(struct cache *cache, struct bio *bio,
> dm_oblock_t oblock, dm_cblock_t cblock)
> {
> @@ -971,23 +979,25 @@ static void writethrough_endio(struct bio *bio)
> }
> /*
> - * FIXME: send in parallel, huge latency as is.
> * When running in writethrough mode we need to send writes to clean blocks
> - * to both the cache and origin devices. In future we'd like to clone the
> - * bio and send them in parallel, but for now we're doing them in
> - * series as this is easier.
> + * to both the cache and origin devices. Clone the bio and send
> them in parallel.
> */
> -static void remap_to_origin_then_cache(struct cache *cache, struct
> bio *bio,
> - dm_oblock_t oblock, dm_cblock_t cblock)
> +static void remap_to_origin_and_cache(struct cache *cache, struct bio *bio,
> + dm_oblock_t oblock, dm_cblock_t cblock)
> {
> - struct per_bio_data *pb = get_per_bio_data(bio, PB_DATA_SIZE_WT);
> + struct bio *origin_bio = bio_clone_fast(bio, GFP_NOIO, cache->bs);
> - pb->cache = cache;
> - pb->cblock = cblock;
> - dm_hook_bio(&pb->hook_info, bio, writethrough_endio, NULL);
> - dm_bio_record(&pb->bio_details, bio);
> + BUG_ON(!origin_bio);
> - remap_to_origin_clear_discard(pb->cache, bio, oblock);
> + bio_chain(origin_bio, bio);
> + /*
> + * Passing false to __remap_to_origin_clear_discard() skips
> + * all code that might use per_bio_data (since clone doesn't have it)
> + */
> + __remap_to_origin_clear_discard(cache, origin_bio, oblock, false);
> + submit_bio(origin_bio);
> +
> + remap_to_cache(cache, bio, cblock);
> }
> /*----------------------------------------------------------------
> @@ -1873,7 +1883,7 @@ static int map_bio(struct cache *cache, struct
> bio *bio, dm_oblock_t block,
> } else {
> if (bio_data_dir(bio) == WRITE && writethrough_mode(&cache->features) &&
> !is_dirty(cache, cblock)) {
> - remap_to_origin_then_cache(cache, bio, block, cblock);
> + remap_to_origin_and_cache(cache, bio, block, cblock);
> accounted_begin(cache, bio);
> } else
> remap_to_cache_dirty(cache, bio, block, cblock);
> @@ -2132,6 +2142,9 @@ static void destroy(struct cache *cache)
> kfree(cache->ctr_args[i]);
> kfree(cache->ctr_args);
> + if (cache->bs)
> + bioset_free(cache->bs);
> +
> kfree(cache);
> }
> @@ -2589,6 +2602,13 @@ static int cache_create(struct cache_args
> *ca, struct cache **result)
> cache->features = ca->features;
> ti->per_io_data_size = get_per_bio_data_size(cache);
> + if (writethrough_mode(&cache->features)) {
> + /* Create bioset for writethrough bios issued to origin */
> + cache->bs = bioset_create(BIO_POOL_SIZE, 0, 0);
> + if (!cache->bs)
> + goto bad;
> + }
> +
> cache->callbacks.congested_fn = cache_is_congested;
> dm_table_add_target_callbacks(ti->table, &cache->callbacks);
> 
> -- 
> 2.26.2
> 

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Sasha Levin]

On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>
>Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9

Hm, what's the issue that this patch addresses? It's not clear from the
commit message.

-- 
Thanks,
Sasha

re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[John Donnelly]

> On Jul 27, 2020, at 2:18 PM, Sasha Levin <sashal@kernel.org> wrote:
> 
> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>> 
>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> 
> Hm, what's the issue that this patch addresses? It's not clear from the
> commit message.
> 
> -- 
> Thanks,
> Sasha

HI Sasha ,

In an off-line conversation I had with Mike , he indicated that :


commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b
Author: Mike Snitzer <snitzer@redhat.com>
Date:   Fri Feb 28 18:00:53 2020 -0500

   dm bio record: save/restore bi_end_io and bi_integrity


commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e
Author: Mike Snitzer <snitzer@redhat.com>
Date:   Fri Feb 28 18:11:53 2020 -0500

   dm integrity: use dm_bio_record and dm_bio_restore


Were picked up  in  "stable" kernels picked up even though 
neither was marked for stable@vger.kernel.org 

Adding this missing  commit :  

 2df3bae9a6543e90042291707b8db0cbfbae9ee9


Completes the series 


Thank you ,


John.

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Sasha Levin]

On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote:
>
>
>> On Jul 27, 2020, at 2:18 PM, Sasha Levin <sashal@kernel.org> wrote:
>>
>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>>>
>>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>>
>> Hm, what's the issue that this patch addresses? It's not clear from the
>> commit message.
>>
>> --
>> Thanks,
>> Sasha
>
>HI Sasha ,
>
>In an off-line conversation I had with Mike , he indicated that :
>
>
>commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b
>Author: Mike Snitzer <snitzer@redhat.com>
>Date:   Fri Feb 28 18:00:53 2020 -0500
>
>   dm bio record: save/restore bi_end_io and bi_integrity
>
>
>commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e
>Author: Mike Snitzer <snitzer@redhat.com>
>Date:   Fri Feb 28 18:11:53 2020 -0500
>
>   dm integrity: use dm_bio_record and dm_bio_restore
>
>
>Were picked up  in  "stable" kernels picked up even though
>neither was marked for stable@vger.kernel.org
>
>Adding this missing  commit :
>
> 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>
>
>Completes the series

Should we just revert those two commits instead if they're not needed?

-- 
Thanks,
Sasha

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Mike Snitzer]

On Mon, Jul 27 2020 at  4:17pm -0400,
Sasha Levin <sashal@kernel.org> wrote:

> On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote:
> >
> >
> >>On Jul 27, 2020, at 2:18 PM, Sasha Levin <sashal@kernel.org> wrote:
> >>
> >>On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> >>>This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> >>>
> >>>Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> >>
> >>Hm, what's the issue that this patch addresses? It's not clear from the
> >>commit message.
> >>
> >>--
> >>Thanks,
> >>Sasha
> >
> >HI Sasha ,
> >
> >In an off-line conversation I had with Mike , he indicated that :
> >
> >
> >commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b
> >Author: Mike Snitzer <snitzer@redhat.com>
> >Date:   Fri Feb 28 18:00:53 2020 -0500
> >
> >  dm bio record: save/restore bi_end_io and bi_integrity
> >
> >
> >commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e
> >Author: Mike Snitzer <snitzer@redhat.com>
> >Date:   Fri Feb 28 18:11:53 2020 -0500
> >
> >  dm integrity: use dm_bio_record and dm_bio_restore
> >
> >
> >Were picked up  in  "stable" kernels picked up even though
> >neither was marked for stable@vger.kernel.org
> >
> >Adding this missing  commit :
> >
> >2df3bae9a6543e90042291707b8db0cbfbae9ee9
> >
> >
> >Completes the series
> 
> Should we just revert those two commits instead if they're not needed?

I'd be fine with that, exceept I haven't looked to see whether any
other stable commits conflict with reverting them.

But you have my blessing to give it a shot ;)

Mike

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[John Donnelly]

> On Jul 27, 2020, at 3:17 PM, Sasha Levin <sashal@kernel.org> wrote:
> 
> On Mon, Jul 27, 2020 at 02:38:52PM -0500, John Donnelly wrote:
>> 
>> 
>>> On Jul 27, 2020, at 2:18 PM, Sasha Levin <sashal@kernel.org> wrote:
>>> 
>>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>>>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>>>> 
>>>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>>> 
>>> Hm, what's the issue that this patch addresses? It's not clear from the
>>> commit message.
>>> 
>>> --
>>> Thanks,
>>> Sasha
>> 
>> HI Sasha ,
>> 
>> In an off-line conversation I had with Mike , he indicated that :
>> 
>> 
>> commit 1b17159e52bb31f982f82a6278acd7fab1d3f67b
>> Author: Mike Snitzer <snitzer@redhat.com>
>> Date:   Fri Feb 28 18:00:53 2020 -0500
>> 
>>  dm bio record: save/restore bi_end_io and bi_integrity
>> 
>> 
>> commit 248aa2645aa7fc9175d1107c2593cc90d4af5a4e
>> Author: Mike Snitzer <snitzer@redhat.com>
>> Date:   Fri Feb 28 18:11:53 2020 -0500
>> 
>>  dm integrity: use dm_bio_record and dm_bio_restore
>> 
>> 
>> Were picked up  in  "stable" kernels picked up even though
>> neither was marked for stable@vger.kernel.org
>> 
>> Adding this missing  commit :
>> 
>> 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>> 
>> 
>> Completes the series
> 
> Should we just revert those two commits instead if they're not needed?
> 
> -- 
> Thanks,
> Sasha

  As I stated above:

> Fixes: 705559706d62038b74c5088114c1799cf2c9dce8 (dm bio record:
> save/restore bi_end_io and bi_integrity, version 4.14.188)
> 
> 70555970 introduced a mkfs.ext4 hang on a LVM device that has been
> modified with lvconvert --cachemode=writethrough.

  It corrects an issue we discovered in 4.14.188 .    Any other branches those two commits have migrated to will likely have the same regression. 

I am confident linux-4.14.y will  be better off with it ;-) 

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Greg KH]

On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> 
> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9

Now backported, thanks.

greg k-h

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Greg KH]

On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> > This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> > 
> > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> 
> Now backported, thanks.

Nope, it broke the build, I need something that actually works :)

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Mike Snitzer]

On Wed, Jul 29 2020 at  7:55am -0400,
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
> > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> > > This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> > > 
> > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> > 
> > Now backported, thanks.
> 
> Nope, it broke the build, I need something that actually works :)
> 

OK, I'll defer to John Donnelly to get back with you (and rest of
stable@).  He is more invested due to SUSE also having this issue.  I
can put focus to it if John cannot sort this out.

Mike

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[John Donnelly]

On 7/29/20 9:16 AM, Mike Snitzer wrote:
> On Wed, Jul 29 2020 at  7:55am -0400,
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
>> On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
>>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>>>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>>>>
>>>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>>>
>>> Now backported, thanks.
>>
>> Nope, it broke the build, I need something that actually works :)
>>
> 
> OK, I'll defer to John Donnelly to get back with you (and rest of
> stable@).  He is more invested due to SUSE also having this issue.  I
> can put focus to it if John cannot sort this out.
> 
> Mike
> 


Hi.


Thank you for reaching out.

What specifically is broken? . If it that applying 
2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing?

JD.

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Greg KH]

On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote:
> 
> 
> On 7/29/20 9:16 AM, Mike Snitzer wrote:
> > On Wed, Jul 29 2020 at  7:55am -0400,
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
> > > > On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> > > > > This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> > > > > 
> > > > > Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> > > > 
> > > > Now backported, thanks.
> > > 
> > > Nope, it broke the build, I need something that actually works :)
> > > 
> > 
> > OK, I'll defer to John Donnelly to get back with you (and rest of
> > stable@).  He is more invested due to SUSE also having this issue.  I
> > can put focus to it if John cannot sort this out.
> > 
> > Mike
> > 
> 
> 
> Hi.
> 
> 
> Thank you for reaching out.
> 
> What specifically is broken? . If it that applying
> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing?

yes, try it yourself and see!

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[John Donnelly]

> On Jul 30, 2020, at 12:21 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote:
>> 
>> 
>> On 7/29/20 9:16 AM, Mike Snitzer wrote:
>>> On Wed, Jul 29 2020 at  7:55am -0400,
>>> Greg KH <gregkh@linuxfoundation.org> wrote:
>>> 
>>>> On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
>>>>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
>>>>>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
>>>>>> 
>>>>>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
>>>>> 
>>>>> Now backported, thanks.
>>>> 
>>>> Nope, it broke the build, I need something that actually works :)
>>>> 
>>> 
>>> OK, I'll defer to John Donnelly to get back with you (and rest of
>>> stable@).  He is more invested due to SUSE also having this issue.  I
>>> can put focus to it if John cannot sort this out.
>>> 
>>> Mike
>>> 
>> 
>> 
>> Hi.
>> 
>> 
>> Thank you for reaching out.
>> 
>> What specifically is broken? . If it that applying
>> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing?
> 
> yes, try it yourself and see!

 Hi . 

 Yes .  

  2df3bae9a6543e90042291707b8db0cbfbae9ee9

 Needs refactored to work in 4.14.y (now .190) as there is a conflict in arguments as noted in my original submittal ;-) . 
 I also noticed there are warning to functions " defined but not used [-Wunused-function] “  too.

 Do you want another PATCH v2 message  in a new email thread,  or can I  append it to this this thread ?

Please advice.

Thanks.
JD.





 

Re: (resend) [PATCH [linux-4.14.y]] dm cache: submit writethrough writes in parallel to origin and cache

[Greg KH]

On Thu, Jul 30, 2020 at 09:40:32AM -0500, John Donnelly wrote:
> 
> 
> > On Jul 30, 2020, at 12:21 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > On Wed, Jul 29, 2020 at 06:45:46PM -0500, John Donnelly wrote:
> >> 
> >> 
> >> On 7/29/20 9:16 AM, Mike Snitzer wrote:
> >>> On Wed, Jul 29 2020 at  7:55am -0400,
> >>> Greg KH <gregkh@linuxfoundation.org> wrote:
> >>> 
> >>>> On Wed, Jul 29, 2020 at 01:51:19PM +0200, Greg KH wrote:
> >>>>> On Mon, Jul 27, 2020 at 11:00:14AM -0400, Mike Snitzer wrote:
> >>>>>> This mail needs to be saent to stable@vger.kernel.org (now cc'd).
> >>>>>> 
> >>>>>> Greg et al: please backport 2df3bae9a6543e90042291707b8db0cbfbae9ee9
> >>>>> 
> >>>>> Now backported, thanks.
> >>>> 
> >>>> Nope, it broke the build, I need something that actually works :)
> >>>> 
> >>> 
> >>> OK, I'll defer to John Donnelly to get back with you (and rest of
> >>> stable@).  He is more invested due to SUSE also having this issue.  I
> >>> can put focus to it if John cannot sort this out.
> >>> 
> >>> Mike
> >>> 
> >> 
> >> 
> >> Hi.
> >> 
> >> 
> >> Thank you for reaching out.
> >> 
> >> What specifically is broken? . If it that applying
> >> 2df3bae9a6543e90042291707b8db0cbfbae9ee9 to 4.14.y is failing?
> > 
> > yes, try it yourself and see!
> 
>  Hi . 
> 
>  Yes .  
> 
>   2df3bae9a6543e90042291707b8db0cbfbae9ee9
> 
>  Needs refactored to work in 4.14.y (now .190) as there is a conflict in arguments as noted in my original submittal ;-) . 
>  I also noticed there are warning to functions " defined but not used [-Wunused-function] “  too.
> 
>  Do you want another PATCH v2 message  in a new email thread,  or can I  append it to this this thread ?

I do not care, either should be fine.