linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* memblock and bootmem problems if start + size = 4GB
@ 2011-12-19 13:58 Michal Simek
  2011-12-19 16:28 ` Tejun Heo
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2011-12-19 13:58 UTC (permalink / raw)
  To: Andrew Morton, Tejun Heo, Yinghai Lu, Benjamin Herrenschmidt,
	Sam Ravnborg
  Cc: linux-mm, LKML

Hi,

I have reached some problems with memblock and bootmem code for some configurations.
We can completely setup the whole system and all addresses in it.
The problem happens if we place main memory to the end of address space when
mem_start + size reach 4GB limit.

For example:
mem_start      0xF000 0000
mem_size       0x1000 0000 (or better lowmem size)
mem_end        0xFFFF FFFF
start + size 0x1 0000 0000 (u32 limit reached).

I have done some patches which completely remove start + size values from architecture specific
code but I have found some problem in generic code too.

For example in bootmem code where are three places where physaddr + size is used.
I would prefer to retype it to u64 because baseaddr and size don't need to be 2^n.

Is it correct solution? If yes, I will create proper patch.

diff --git a/mm/bootmem.c b/mm/bootmem.c
index 1a77012..45a691a 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -371,7 +371,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
         kmemleak_free_part(__va(physaddr), size);

         start = PFN_UP(physaddr);
-       end = PFN_DOWN(physaddr + size);
+       end = PFN_DOWN((u64)physaddr + (u64)size);

         mark_bootmem_node(pgdat->bdata, start, end, 0, 0);
  }
@@ -414,7 +414,7 @@ int __init reserve_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
         unsigned long start, end;

         start = PFN_DOWN(physaddr);
-       end = PFN_UP(physaddr + size);
+       end = PFN_UP((u64)physaddr + (u64)size);

         return mark_bootmem_node(pgdat->bdata, start, end, 1, flags);
  }
@@ -435,7 +435,7 @@ int __init reserve_bootmem(unsigned long addr, unsigned long size,
         unsigned long start, end;

         start = PFN_DOWN(addr);
-       end = PFN_UP(addr + size);
+       end = PFN_UP((u64)addr + (u64)size);

         return mark_bootmem(start, end, 1, flags);
  }



The similar problem is with memblock code.

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index e6b843e..55d5279 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -127,7 +127,7 @@ static inline unsigned long memblock_region_memory_base_pfn(const struct membloc
   */
  static inline unsigned long memblock_region_memory_end_pfn(const struct memblock_region *reg)
  {
-       return PFN_DOWN(reg->base + reg->size);
+       return PFN_DOWN((u64)reg->base + (u64)reg->size);
  }

  /**
@@ -145,7 +145,7 @@ static inline unsigned long memblock_region_reserved_base_pfn(const struct membl
   */
  static inline unsigned long memblock_region_reserved_end_pfn(const struct memblock_region *reg)
  {
-       return PFN_UP(reg->base + reg->size);
+       return PFN_UP((u64)reg->base + (u64)reg->size);
  }

  #define for_each_memblock(memblock_type, region)                                       \


Plus fixing two conditions in memblock_find_base for the same reasons.

diff --git a/mm/memblock.c b/mm/memblock.c
index 84bec49..6c443bd 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -131,10 +131,10 @@ static phys_addr_t __init_memblock memblock_find_base(phys_addr_t size,

                 if (memblocksize < size)
                         continue;
-               if ((memblockbase + memblocksize) <= start)
+               if ((memblockbase + memblocksize - 1) <= start)
                         break;
                 bottom = max(memblockbase, start);
-               top = min(memblockbase + memblocksize, end);
+               top = min(memblockbase + memblocksize - 1, end);
                 if (bottom >= top)
                         continue;
                 found = memblock_find_region(bottom, top, size, align);


Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-19 13:58 memblock and bootmem problems if start + size = 4GB Michal Simek
@ 2011-12-19 16:28 ` Tejun Heo
  2011-12-20  9:19   ` Michal Simek
  0 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2011-12-19 16:28 UTC (permalink / raw)
  To: Michal Simek
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Hello, Michal.

On Mon, Dec 19, 2011 at 02:58:13PM +0100, Michal Simek wrote:
> I have reached some problems with memblock and bootmem code for some configurations.
> We can completely setup the whole system and all addresses in it.
> The problem happens if we place main memory to the end of address space when
> mem_start + size reach 4GB limit.
> 
> For example:
> mem_start      0xF000 0000
> mem_size       0x1000 0000 (or better lowmem size)
> mem_end        0xFFFF FFFF
> start + size 0x1 0000 0000 (u32 limit reached).
> 
> I have done some patches which completely remove start + size values from architecture specific
> code but I have found some problem in generic code too.
> 
> For example in bootmem code where are three places where physaddr + size is used.
> I would prefer to retype it to u64 because baseaddr and size don't need to be 2^n.
> 
> Is it correct solution? If yes, I will create proper patch.

Yeah, that's an inherent problem in using [) ranges but I think
chopping off the last page probably is simpler and more robust
solution.  Currently, memblock_add_region() would simply ignore if
address range overflows but making it just ignore the last page is
several lines of addition.  Wouldn't that be effective enough while
staying very simple?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-19 16:28 ` Tejun Heo
@ 2011-12-20  9:19   ` Michal Simek
  2011-12-29 13:44     ` Michal Simek
  2011-12-29 15:58     ` Tejun Heo
  0 siblings, 2 replies; 9+ messages in thread
From: Michal Simek @ 2011-12-20  9:19 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Michal Simek, Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt,
	Sam Ravnborg, linux-mm, LKML

Hi Tejun,

> On Mon, Dec 19, 2011 at 02:58:13PM +0100, Michal Simek wrote:
>> I have reached some problems with memblock and bootmem code for some configurations.
>> We can completely setup the whole system and all addresses in it.
>> The problem happens if we place main memory to the end of address space when
>> mem_start + size reach 4GB limit.
>>
>> For example:
>> mem_start      0xF000 0000
>> mem_size       0x1000 0000 (or better lowmem size)
>> mem_end        0xFFFF FFFF
>> start + size 0x1 0000 0000 (u32 limit reached).
>>
>> I have done some patches which completely remove start + size values from architecture specific
>> code but I have found some problem in generic code too.
>>
>> For example in bootmem code where are three places where physaddr + size is used.
>> I would prefer to retype it to u64 because baseaddr and size don't need to be 2^n.
>>
>> Is it correct solution? If yes, I will create proper patch.
> 
> Yeah, that's an inherent problem in using [) ranges but I think
> chopping off the last page probably is simpler and more robust
> solution.  Currently, memblock_add_region() would simply ignore if
> address range overflows but making it just ignore the last page is
> several lines of addition.  Wouldn't that be effective enough while
> staying very simple?

The main problem is with PFN_DOWN/UP macros and it is in __init section.
The result will be definitely u32 type (for 32bit archs) anyway and seems to me
better solution than ignoring the last page.

Is there any internal kernel test code to test all pages - try to allocate/use/test it?
It will be especially good to do so on the last page to see if there is any problem or not.

That two conditions in memblock should be ok.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-20  9:19   ` Michal Simek
@ 2011-12-29 13:44     ` Michal Simek
  2011-12-29 15:58     ` Tejun Heo
  1 sibling, 0 replies; 9+ messages in thread
From: Michal Simek @ 2011-12-29 13:44 UTC (permalink / raw)
  Cc: Tejun Heo, Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt,
	Sam Ravnborg, linux-mm, LKML

Michal Simek wrote:
> Hi Tejun,
> 
>> On Mon, Dec 19, 2011 at 02:58:13PM +0100, Michal Simek wrote:
>>> I have reached some problems with memblock and bootmem code for some 
>>> configurations.
>>> We can completely setup the whole system and all addresses in it.
>>> The problem happens if we place main memory to the end of address 
>>> space when
>>> mem_start + size reach 4GB limit.
>>>
>>> For example:
>>> mem_start      0xF000 0000
>>> mem_size       0x1000 0000 (or better lowmem size)
>>> mem_end        0xFFFF FFFF
>>> start + size 0x1 0000 0000 (u32 limit reached).
>>>
>>> I have done some patches which completely remove start + size values 
>>> from architecture specific
>>> code but I have found some problem in generic code too.
>>>
>>> For example in bootmem code where are three places where physaddr + 
>>> size is used.
>>> I would prefer to retype it to u64 because baseaddr and size don't 
>>> need to be 2^n.
>>>
>>> Is it correct solution? If yes, I will create proper patch.
>>
>> Yeah, that's an inherent problem in using [) ranges but I think
>> chopping off the last page probably is simpler and more robust
>> solution.  Currently, memblock_add_region() would simply ignore if
>> address range overflows but making it just ignore the last page is
>> several lines of addition.  Wouldn't that be effective enough while
>> staying very simple?
> 
> The main problem is with PFN_DOWN/UP macros and it is in __init section.
> The result will be definitely u32 type (for 32bit archs) anyway and 
> seems to me
> better solution than ignoring the last page.
> 
> Is there any internal kernel test code to test all pages - try to 
> allocate/use/test it?
> It will be especially good to do so on the last page to see if there is 
> any problem or not.
> 
> That two conditions in memblock should be ok.

Tejun and Andrew: any other comment?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-20  9:19   ` Michal Simek
  2011-12-29 13:44     ` Michal Simek
@ 2011-12-29 15:58     ` Tejun Heo
  2011-12-29 16:46       ` Michal Simek
  1 sibling, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2011-12-29 15:58 UTC (permalink / raw)
  To: Michal Simek
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Hello,

On Tue, Dec 20, 2011 at 10:19:18AM +0100, Michal Simek wrote:
> >Yeah, that's an inherent problem in using [) ranges but I think
> >chopping off the last page probably is simpler and more robust
> >solution.  Currently, memblock_add_region() would simply ignore if
> >address range overflows but making it just ignore the last page is
> >several lines of addition.  Wouldn't that be effective enough while
> >staying very simple?
> 
> The main problem is with PFN_DOWN/UP macros and it is in __init section.
> The result will be definitely u32 type (for 32bit archs) anyway and seems to me
> better solution than ignoring the last page.

Other than being able to use one more 4k page, is there any other
benefit?  Maybe others had different experiences but in my exprience
trying to extend range coverages - be it stack top/end pointers,
address ranges or whatnot - using [] ranges or special flag usually
ended up adding complexity while adding almost nothing tangible.  On
extreme cases, people even carry separate valid flag to use %NULL as
valid address, which is pretty silly, IMHO.  So, unless there's some
benefit that I'm missing, I still think it's an overkill.  It's more
complex and difficult to test and verify.  Why bother for a single
page?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-29 15:58     ` Tejun Heo
@ 2011-12-29 16:46       ` Michal Simek
  2011-12-29 17:07         ` Tejun Heo
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2011-12-29 16:46 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Tejun Heo wrote:
> Hello,
> 
> On Tue, Dec 20, 2011 at 10:19:18AM +0100, Michal Simek wrote:
>>> Yeah, that's an inherent problem in using [) ranges but I think
>>> chopping off the last page probably is simpler and more robust
>>> solution.  Currently, memblock_add_region() would simply ignore if
>>> address range overflows but making it just ignore the last page is
>>> several lines of addition.  Wouldn't that be effective enough while
>>> staying very simple?
>> The main problem is with PFN_DOWN/UP macros and it is in __init section.
>> The result will be definitely u32 type (for 32bit archs) anyway and seems to me
>> better solution than ignoring the last page.
> 
> Other than being able to use one more 4k page, is there any other
> benefit? Maybe others had different experiences but in my exprience
> trying to extend range coverages - be it stack top/end pointers,
> address ranges or whatnot - using [] ranges or special flag usually
> ended up adding complexity while adding almost nothing tangible.

First of all I don't like to use your term "extend range coverages".
We don't want to extend any ranges - we just wanted to place memory to the end
of address space and be able to work with. It is limitation which should be fixed somehow.
And I would expect that PFN_XX(base + size) will be in u32 range.

Probably the best solution will be to use PFN macro in one place and do not covert
addresses in common code.

+ change parameters in bootmem code because some arch do
free_bootmem_node(..., PFN_PHYS(), ...)
and
reserve_bootmem_node(..., PFN_PHYS(), ...)

and then in that functions(free/reseve_bootmem_code) are used PFN_DOWN/PFN_UP macros.
If alignment is handled by architecture code (which I believe is) then should be possible to change parameters.

For example:
void __init free_bootmem_node(pg_data_t *pgdat, unsigned long start_pfn,
			      unsigned long end_pfn)

int __init reserve_bootmem_node(pg_data_t *pgdat, unsigned long start_pfn,
				 unsigned long end_pfn, int flags)

Is there any reason to use use physical addresses instead of pfns in bootmem code?

 >  On
> extreme cases, people even carry separate valid flag to use %NULL as
> valid address, which is pretty silly, IMHO.  So, unless there's some
> benefit that I'm missing, I still think it's an overkill.  It's more
> complex and difficult to test and verify.  Why bother for a single
> page?

Where do you think this page should be placed? In common code or in architecture memory
code where one page from the top of 4G should be subtract?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-29 16:46       ` Michal Simek
@ 2011-12-29 17:07         ` Tejun Heo
  2011-12-30  7:58           ` Michal Simek
  0 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2011-12-29 17:07 UTC (permalink / raw)
  To: Michal Simek
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Hello,

On Thu, Dec 29, 2011 at 05:46:18PM +0100, Michal Simek wrote:
> First of all I don't like to use your term "extend range coverages".
> We don't want to extend any ranges - we just wanted to place memory to the end
> of address space and be able to work with.

It is, as long as we use address ranges.  Either we can express length
of zero or include the last address.

> It is limitation which should be fixed somehow.
> And I would expect that PFN_XX(base + size) will be in u32 range.
>
> Probably the best solution will be to use PFN macro in one place and
> do not covert addresses in common code.
> 
> + change parameters in bootmem code because some arch do
> free_bootmem_node(..., PFN_PHYS(), ...)
> and
> reserve_bootmem_node(..., PFN_PHYS(), ...)

So now we're talking about a lot of code just for ONE page and
regardless of the representation in the memblock or other memory
management code, I think trying to use that page is fundamentally a
bad idea.  There are a lot of places in the kernel where phys_addr_t
is used.  Using that one last page risks obscure overflow bug if any
of them is using [start,end) ranges and bugs triggered such way would
be extremely difficult to track down.  It doesn't make any sense to do
that for that one last page.  It's less severe but in the same vein as
trying to use %NULL as a valid address.  It's an absurdly silly
tradeoff.

So, FWIW, I think that is a horrible idea.

> >  On
> >extreme cases, people even carry separate valid flag to use %NULL as
> >valid address, which is pretty silly, IMHO.  So, unless there's some
> >benefit that I'm missing, I still think it's an overkill.  It's more
> >complex and difficult to test and verify.  Why bother for a single
> >page?
> 
> Where do you think this page should be placed? In common code or in architecture memory
> code where one page from the top of 4G should be subtract?

With the pending updates to memblock code in tip scheduled for the
coming merge window, I *think* it would be a single (or a few) line
change in memblock_add_region() where it checks for overflow.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-29 17:07         ` Tejun Heo
@ 2011-12-30  7:58           ` Michal Simek
  2011-12-30 17:45             ` Tejun Heo
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2011-12-30  7:58 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Tejun Heo wrote:
> Hello,
> 
> On Thu, Dec 29, 2011 at 05:46:18PM +0100, Michal Simek wrote:
>> First of all I don't like to use your term "extend range coverages".
>> We don't want to extend any ranges - we just wanted to place memory to the end
>> of address space and be able to work with.
> 
> It is, as long as we use address ranges.  Either we can express length
> of zero or include the last address.
> 
>> It is limitation which should be fixed somehow.
>> And I would expect that PFN_XX(base + size) will be in u32 range.
>>
>> Probably the best solution will be to use PFN macro in one place and
>> do not covert addresses in common code.
>>
>> + change parameters in bootmem code because some arch do
>> free_bootmem_node(..., PFN_PHYS(), ...)
>> and
>> reserve_bootmem_node(..., PFN_PHYS(), ...)
> 
> So now we're talking about a lot of code just for ONE page and
> regardless of the representation in the memblock or other memory
> management code, I think trying to use that page is fundamentally a
> bad idea.  There are a lot of places in the kernel where phys_addr_t
> is used. 

I haven't said to replace phys_addr_t!
My point was something like this (just as example on parisc and free_bootmem_node).
The problematic part is kmemleak code which could be good reason not to change it.

Thanks,
Michal


diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index 82f364e..b83ee32 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -291,8 +291,8 @@ static void __init setup_bootmem(void)
                                                 start_pfn,
                                                 (start_pfn + npages) );
                 free_bootmem_node(NODE_DATA(i),
-                                 (start_pfn << PAGE_SHIFT),
-                                 (npages << PAGE_SHIFT) );
+                                 start_pfn,
+                                 npages);
                 bootmap_pfn += (bootmap_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
                 if ((start_pfn + npages) > max_pfn)
                         max_pfn = start_pfn + npages;



diff --git a/mm/bootmem.c b/mm/bootmem.c
index 45a691a..dfbfc47 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -363,17 +363,12 @@ static int __init mark_bootmem(unsigned long start, unsigned long end,
   *
   * The range must reside completely on the specified node.
   */
-void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
-                             unsigned long size)
+void __init free_bootmem_node(pg_data_t *pgdat, unsigned long startpfn,
+                             unsigned long endpfn)
  {
-       unsigned long start, end;
-
-       kmemleak_free_part(__va(physaddr), size);
+       kmemleak_free_part(__va(startpfn << PAGE_SHIFT), (endpfn - startpfn) << PAGE_SHIFT);

-       start = PFN_UP(physaddr);
-       end = PFN_DOWN((u64)physaddr + (u64)size);
-
-       mark_bootmem_node(pgdat->bdata, start, end, 0, 0);
+       mark_bootmem_node(pgdat->bdata, startpfn, endpfn, 0, 0);
  }




-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: memblock and bootmem problems if start + size = 4GB
  2011-12-30  7:58           ` Michal Simek
@ 2011-12-30 17:45             ` Tejun Heo
  0 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2011-12-30 17:45 UTC (permalink / raw)
  To: monstr
  Cc: Andrew Morton, Yinghai Lu, Benjamin Herrenschmidt, Sam Ravnborg,
	linux-mm, LKML

Hello,

On Fri, Dec 30, 2011 at 4:58 PM, Michal Simek <monstr@monstr.eu> wrote:
> I haven't said to replace phys_addr_t!
> My point was something like this (just as example on parisc and
> free_bootmem_node).
> The problematic part is kmemleak code which could be good reason not to
> change it.

I think it's still a bad idea and you haven't provided any
justification for it.  Think about it - any user which uses pa() may
get that last page and if that user is using [start,end) range, it may
overflow.  It doesn't even matter how you implement it.  I just can't
understand why you obsess about that last page.  It doesn't matter.
Just add those few lines to exclude the last single page and be done
with it.  Unless you're gonna provide rationale for why adding such
risk and more complexity makes sense for that single last page which
most BIOSes wouldn't even map, I don't really think this thread is
going anywhere.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-12-30 17:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-19 13:58 memblock and bootmem problems if start + size = 4GB Michal Simek
2011-12-19 16:28 ` Tejun Heo
2011-12-20  9:19   ` Michal Simek
2011-12-29 13:44     ` Michal Simek
2011-12-29 15:58     ` Tejun Heo
2011-12-29 16:46       ` Michal Simek
2011-12-29 17:07         ` Tejun Heo
2011-12-30  7:58           ` Michal Simek
2011-12-30 17:45             ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).