Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1161893 > unrolled thread
| Started by | Christoph Lameter <cl@linux.com> |
|---|---|
| First post | 2015-06-10 03:20 +0200 |
| Last post | 2015-06-10 04:10 +0200 |
| Articles | 5 — 3 participants |
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.
Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions Christoph Lameter <cl@linux.com> - 2015-06-10 03:20 +0200
Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions Andrew Morton <akpm@linux-foundation.org> - 2015-06-10 03:50 +0200
Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions Christoph Lameter <cl@linux.com> - 2015-06-10 04:10 +0200
Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions Andrew Morton <akpm@linux-foundation.org> - 2015-06-10 04:20 +0200
Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-06-10 04:10 +0200
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2015-06-10 03:20 +0200 |
| Subject | Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions |
| Message-ID | <pzCQp-7qt-5@gated-at.bofh.it> |
On Tue, 9 Jun 2015, Andrew Morton wrote: > Well I like it, even though it's going to cause a zillion little cleanup > patches. > > checkpatch already has a "kfree(NULL) is safe and this check is > probably not required" test so I guess Joe will need to get busy ;) > > I'll park these patches until after 4.1 is released - it's getting to > that time... Why do this at all? I understand that kfree/kmem_cache_free can take a null pointer but this is the destruction of a cache and it usually requires multiple actions to clean things up and these actions have to be properly sequenced. All other processors have to stop referencing this cache before it can be destroyed. I think failing if someone does something strange like doing cache destruction with a NULL pointer is valuable. -- 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]
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2015-06-10 03:50 +0200 |
| Subject | Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions |
| Message-ID | <pzDjs-7Zy-3@gated-at.bofh.it> |
| In reply to | #1161893 |
On Tue, 9 Jun 2015 20:11:25 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote: > On Tue, 9 Jun 2015, Andrew Morton wrote: > > > Well I like it, even though it's going to cause a zillion little cleanup > > patches. > > > > checkpatch already has a "kfree(NULL) is safe and this check is > > probably not required" test so I guess Joe will need to get busy ;) > > > > I'll park these patches until after 4.1 is released - it's getting to > > that time... > > Why do this at all? For the third time: because there are approx 200 callsites which are already doing it. > I understand that kfree/kmem_cache_free can take a > null pointer but this is the destruction of a cache and it usually > requires multiple actions to clean things up and these actions have to be > properly sequenced. All other processors have to stop referencing this > cache before it can be destroyed. I think failing if someone does > something strange like doing cache destruction with a NULL pointer is > valuable. More than half of the kmem_cache_destroy() callsites are declining that value by open-coding the NULL test. That's reality and we should recognize it. -- 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]
| From | Christoph Lameter <cl@linux.com> |
|---|---|
| Date | 2015-06-10 04:10 +0200 |
| Message-ID | <pzDCO-9p-3@gated-at.bofh.it> |
| In reply to | #1161897 |
On Tue, 9 Jun 2015, Andrew Morton wrote: > > Why do this at all? > > For the third time: because there are approx 200 callsites which are > already doing it. Did some grepping and I did see some call sites that do this but the majority has to do other processing as well. 200 call sites? Do we have that many uses of caches? Typical prod system have ~190 caches active and the merging brings that down to half of that. > More than half of the kmem_cache_destroy() callsites are declining that > value by open-coding the NULL test. That's reality and we should recognize > it. Well that may just indicate that we need to have a look at those callsites and the reason there to use a special cache at all. If the cache is just something that kmalloc can provide then why create a special cache. On the other hand if something special needs to be accomplished then it would make sense to have special processing on kmem_cache_destroy. -- 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]
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2015-06-10 04:20 +0200 |
| Subject | Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions |
| Message-ID | <pzDMu-re-9@gated-at.bofh.it> |
| In reply to | #1161904 |
On Tue, 9 Jun 2015 21:00:58 -0500 (CDT) Christoph Lameter <cl@linux.com> wrote: > On Tue, 9 Jun 2015, Andrew Morton wrote: > > > > Why do this at all? > > > > For the third time: because there are approx 200 callsites which are > > already doing it. > > Did some grepping and I did see some call sites that do this but the > majority has to do other processing as well. > > 200 call sites? Do we have that many uses of caches? Typical prod system > have ~190 caches active and the merging brings that down to half of that. I didn't try terribly hard. z:/usr/src/linux-4.1-rc7> grep -r -C1 kmem_cache_destroy . | grep "if [(]" | wc -l 158 It's a lot, anyway. > > More than half of the kmem_cache_destroy() callsites are declining that > > value by open-coding the NULL test. That's reality and we should recognize > > it. > > Well that may just indicate that we need to have a look at those > callsites and the reason there to use a special cache at all. This makes no sense. Go look at the code. drivers/staging/lustre/lustre/llite/super25.c, for example. It's all in the basic unwind/recover/exit code. > If the cache > is just something that kmalloc can provide then why create a special > cache. On the other hand if something special needs to be accomplished > then it would make sense to have special processing on kmem_cache_destroy. This has nothing to do with anything. We're talking about a basic "if I created this cache then destroy it" operation. It's a common pattern. mm/ exists to serve client code and as a lot of client code is doing this, we should move it into mm/ so as to serve client code better. -- 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]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2015-06-10 04:10 +0200 |
| Subject | Re: [RFC][PATCH 0/5] do not dereference NULL pools in pools' destroy() functions |
| Message-ID | <pzDCO-9p-1@gated-at.bofh.it> |
| In reply to | #1161893 |
On (06/09/15 20:11), Christoph Lameter wrote: > On Tue, 9 Jun 2015, Andrew Morton wrote: > > > Well I like it, even though it's going to cause a zillion little cleanup > > patches. > > > > checkpatch already has a "kfree(NULL) is safe and this check is > > probably not required" test so I guess Joe will need to get busy ;) > > > > I'll park these patches until after 4.1 is released - it's getting to > > that time... > > Why do this at all? this makes things less fragile. > I understand that kfree/kmem_cache_free can take a > null pointer but this is the destruction of a cache and it usually > requires multiple actions to clean things up and these actions have to be > properly sequenced. All other processors have to stop referencing this > cache before it can be destroyed. >I think failing well, it's not just `failing', it's a NULL pointer deref. > if someone does something strange like doing cache destruction with a > NULL pointer is valuable. > a missing check is not `something strange'. it's just happening. (a very quick google search) http://help.lockergnome.com/linux/PATCH-dlm-NULL-dereference-failure-kmem_cache_create--ftopict555436.html http://linux-kernel.2935.n7.nabble.com/PATCH-2-6-30-rc6-Remove-kmem-cache-destroy-in-s3c24xx-dma-init-td460417.html etc. -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