On Sun, Aug 29, 2021 at 03:11:22PM +0000, Al Viro wrote: > On Sun, Aug 29, 2021 at 02:25:29PM +0000, Caleb D.S. Brzezinski wrote: > > Implement the main msdos_format_name() filename cache. If used as a > > module, all memory allocated for the cache is freed when the module is > > de-registered. > > > > Signed-off-by: Caleb D.S. Brzezinski <calebdsb@xxxxxxxxxxxxxx> > > --- > > fs/fat/namei_msdos.c | 35 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c > > index 7561674b1..f9d4f63c3 100644 > > --- a/fs/fat/namei_msdos.c > > +++ b/fs/fat/namei_msdos.c > > @@ -124,6 +124,16 @@ static int msdos_format_name(const unsigned char *name, int len, > > unsigned char *walk; > > unsigned char c; > > int space; > > + u64 hash; > > + struct msdos_name_node *node; > > + > > + /* check if the name is already in the cache */ > > + > > + hash = msdos_fname_hash(name); > > + if (find_fname_in_cache(res, hash)) > > + return 0; > > Huh? How could that possibly work, seeing that > * your hash function only looks at the first 8 characters > * your find_fname_in_cache() assumes that hash collisions > are impossible, which is... unlikely, considering the nature of > that hash function > * find_fname_in_cache(res, hash) copies at most 8 characters > into res in case of match. Where does the extension come from? > > Out of curiosity, how have you tested that thing? While we are at it, your "fast path" doesn't even look at opts argument...
