On Tue, Mar 04, 2025 at 03:25:26PM +0100, Vlastimil Babka wrote:
> On 3/4/25 13:18, Lilith Gkini wrote:
> > On Tue, Mar 04, 2025 at 12:20:03PM +0100, Vlastimil Babka wrote:
> > I was also thinking of fixing two lines to adhere to the "Breaking long
> > lines and strings" (2) from the coding-style.
>
> Hm AFAIK checkpatch was adjusted to only warn at 100 lines. While the style
> document wasn't updated, we can leave such a small excess with no change.
Yeah, it didn't complain about it, I noticed it while having multiple
windows open with the diffs and all.
> > ---
> > mm/slub.c | 24 +++++++++++++++++-------
> > 1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 1f50129dcfb3..e06b88137705 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1427,7 +1427,7 @@ static int check_slab(struct kmem_cache *s, struct slab *slab)
> > * Determine if a certain object in a slab is on the freelist. Must hold the
> > * slab lock to guarantee that the chains are in a consistent state.
> > */
> > -static int on_freelist(struct kmem_cache *s, struct slab *slab, void *search)
> > +static bool on_freelist(struct kmem_cache *s, struct slab *slab, void *search)
> > {
> > int nr = 0;
> > void *fp;
> > @@ -1437,38 +1437,48 @@ static int on_freelist(struct kmem_cache *s, struct slab *slab, void *search)
> > fp = slab->freelist;
> > while (fp && nr <= slab->objects) {
> > if (fp == search)
> > - return 1;
> > + return true;
> > if (!check_valid_pointer(s, slab, fp)) {
> > if (object) {
> > object_err(s, slab, object,
> > "Freechain corrupt");
> > set_freepointer(s, object, NULL);
> > + break;
> > } else {
> > slab_err(s, slab, "Freepointer corrupt");
> > slab->freelist = NULL;
> > slab->inuse = slab->objects;
> > slab_fix(s, "Freelist cleared");
> > - return 0;
> > + return false;
> > }
> > - break;
> > }
> > object = fp;
> > fp = get_freepointer(s, object);
> > nr++;
> > }
> >
> > - max_objects = order_objects(slab_order(slab), s->size);
> > + if (fp != NULL && nr > slab->objects) {
>
> In case nr > slab->objects we already know fp can't be NULL, no? So we don't
> have to test it?
...Yeah. All these different diffs got me confused. What a mess.
I just tested it in a debugger. That fp null check isn't necessary.
I'll send the full patch tomorrow or something, when I check it again
with a clear head. I dont want to do any mistakes in the actual patch.
> > I do have to note that the last slab_err is of length 81 with my change,
> > but it looks fine. If that one extra character is unacceptable let me
> > know so I can change it to something else.
> > Or if you think it's completely unnecessary I could leave it as it was
> > in the first place.
>
> Yeah can leave it.
>
Alright, I wont include the line breaks in the patch then! I'll leave it as it
was.
