Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1182125 > unrolled thread

Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile Christoph Hellwig <hch@infradead.org> - 2015-07-11 12:10 +0200
    Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile Christoph Hellwig <hch@infradead.org> - 2015-07-13 08:40 +0200
      Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-07-13 09:00 +0200
        Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile Christoph Hellwig <hch@infradead.org> - 2015-07-13 11:10 +0200
          Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-07-13 11:40 +0200

#1182125 — Re: [PATCH 0/2] mm/shrinker: make unregister_shrinker() less fragile

On Sat, Jul 11, 2015 at 11:51:53AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> Shrinker API does not handle nicely unregister_shrinker() on a not-registered
> ->shrinker. Looking at shrinker users, they all have to
> (a) carry on some sort of a flag to make sure that "unregister_shrinker()"
> will not blow up later
> (b) be fishy (potentially can Oops)
> (c) access private members `struct shrinker' (e.g. `shrink.list.next')

Ayone who does that is broken.  You just need to have clear init (with
proper unwinding) and exit functions and order things properly.  It
works like most register/unregister calls and should stay that way.

Maye you you should ty to explain what practical problem you're seeing
to start with.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1182539

On Sun, Jul 12, 2015 at 11:47:32AM +0900, Sergey Senozhatsky wrote:
> Yes, but the main difference here is that it seems that shrinker users
> don't tend to treat shrinker registration failures as fatal errors and
> just continue with shrinker functionality disabled. And it makes sense.
> 
> (copy paste from https://lkml.org/lkml/2015/7/9/751)
> 

I hearily disagree.  It's not any less critical than other failures.

The right way forward is to handle register failure properly.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1182546

On (07/12/15 23:33), Christoph Hellwig wrote:
> On Sun, Jul 12, 2015 at 11:47:32AM +0900, Sergey Senozhatsky wrote:
> > Yes, but the main difference here is that it seems that shrinker users
> > don't tend to treat shrinker registration failures as fatal errors and
> > just continue with shrinker functionality disabled. And it makes sense.
> > 
> > (copy paste from https://lkml.org/lkml/2015/7/9/751)
> > 
> 
> I hearily disagree.  It's not any less critical than other failures.

Why? In some sense, shrinker callbacks are just a way to be nice.
No one writes a driver just to be able to handle shrinker calls. An
ability to react to those calls is just additional option; it does
not directly affect or limit driver's functionality (at least, it
really should not).

> The right way forward is to handle register failure properly.

In other words, to
 (a) keep a flag to signify that register was not successful
or
 (b) look at ->shrinker.list.next or ->nr_deferred
or
 (c) treat register failures as critical errors. (I sort of
     disagree with you here).

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1182617

On Mon, Jul 13, 2015 at 03:52:53PM +0900, Sergey Senozhatsky wrote:
> Why? In some sense, shrinker callbacks are just a way to be nice.
> No one writes a driver just to be able to handle shrinker calls. An
> ability to react to those calls is just additional option; it does
> not directly affect or limit driver's functionality (at least, it
> really should not).

No, they are not just nice.  They are a fundamental part of memory
management and required to reclaim (often large) amounts of memory.

Nevermind that we don't ignore any other registration time error in
the kernel.

> > The right way forward is to handle register failure properly.
> 
> In other words, to
>  (a) keep a flag to signify that register was not successful
> or
>  (b) look at ->shrinker.list.next or ->nr_deferred
> or
>  (c) treat register failures as critical errors. (I sort of
>      disagree with you here).

The only important part is here is (c).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1182646

On (07/13/15 02:03), Christoph Hellwig wrote:
> On Mon, Jul 13, 2015 at 03:52:53PM +0900, Sergey Senozhatsky wrote:
> > Why? In some sense, shrinker callbacks are just a way to be nice.
> > No one writes a driver just to be able to handle shrinker calls. An
> > ability to react to those calls is just additional option; it does
> > not directly affect or limit driver's functionality (at least, it
> > really should not).
> 
> No, they are not just nice.  They are a fundamental part of memory
> management and required to reclaim (often large) amounts of memory.

Yes. 'Nice' used in a sense that drivers have logic to release the
memory anyway; mm asks volunteers (the drivers that have registered
shrinker callbacks) to release some spare/wasted/etc. when things
are getting tough (the drivers are not aware of that in general).
This is surely important to mm, not to the driver though -- it just
agrees to be 'nice', but even not expected to release any memory at
all (IOW, this is not a contract).

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web