LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kari Argillander <kari.argillander@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
	ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] fs/ntfs3: Use kmalloc_array over kmalloc with multiply
Date: Wed, 1 Sep 2021 16:24:48 +0300	[thread overview]
Message-ID: <20210901132448.mgkkyvqszc7lsnwy@kari-VirtualBox> (raw)
In-Reply-To: <5197939870d1867dd2131a8fdff8842b777c6016.camel@perches.com>

On Tue, Aug 31, 2021 at 07:40:58PM -0700, Joe Perches wrote:
> On Tue, 2021-08-31 at 21:15 +0300, Kari Argillander wrote:
> > If we do not use kmalloc_array we get checkpatch warning. It is also
> > little safer if something goes wrong with coding.
> []
> > diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> []
> > @@ -707,7 +707,7 @@ static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx,
> >  		u16 *ptr;
> >  		int new_slots = ALIGN(2 * nslots, 8);
> >  
> > 
> > -		ptr = kmalloc(sizeof(u16) * new_slots, GFP_NOFS);
> > +		ptr = kmalloc_array(new_slots, sizeof(u16), GFP_NOFS);
> >  		if (ptr)
> >  			memcpy(ptr, offs, sizeof(u16) * max_idx);
> 
> This multiplication could also overflow.
> 
> Maybe use krealloc?

Seems to fit lot better here. But as I was watching this it seems that
we do not even need to resize this array. It is quite costly operation
compared to what entry compare cost.

We just need to compare it and if entry diff > 0 then we start entry
table again from zero without need resize array. It may be that we do
not even need to allocate memory. We can probably survive with stack,
but let's think that later.

This can also speed up search a lot. It is quite odd that we always fill
whole table and will not never check if we are over. Worst case is very
very bad. With this change this will be more like jump search but I
think it will be good in this case because we won't need memory allocation.

Thanks Joe.


  reply	other threads:[~2021-09-01 13:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31 18:15 [PATCH 0/5] fs/ntfs3: Fix various styling issues Kari Argillander
2021-08-31 18:15 ` [PATCH 1/5] fs/ntfs3: Use kmalloc_array over kmalloc with multiply Kari Argillander
2021-09-01  2:40   ` Joe Perches
2021-09-01 13:24     ` Kari Argillander [this message]
2021-09-02 14:03     ` David Laight
2021-08-31 18:15 ` [PATCH 2/5] fs/ntfs3: Use consistent spacing around '+' Kari Argillander
2021-09-02  4:22   ` Joe Perches
2021-09-02  8:10     ` Kari Argillander
2021-09-07 16:23       ` Konstantin Komarov
2021-09-07 16:25         ` Kari Argillander
2021-08-31 18:15 ` [PATCH 3/5] fs/ntfs3: Place Comparisons constant right side of the test Kari Argillander
2021-08-31 18:15 ` [PATCH 4/5] fs/ntfs3: Remove braces from single statment block Kari Argillander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210901132448.mgkkyvqszc7lsnwy@kari-VirtualBox \
    --to=kari.argillander@gmail.com \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --subject='Re: [PATCH 1/5] fs/ntfs3: Use kmalloc_array over kmalloc with multiply' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).