From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relayaws-01.paragon-software.com (relayaws-01.paragon-software.com [35.157.23.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AE09E2D8390 for ; Tue, 24 Feb 2026 18:50:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=35.157.23.187 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771959046; cv=none; b=UXu90G3GbgtuTrsLLDJh8qRoj8DlTGZrEsQplPo8VDdsfQaMfVXQ7DWurDK+NvtF9wREchVcuM9CYaRsMq3Ls7F967g7lJcs6gLPOyZmEF7XI9jdpaqg+0A337rH0NanDN9HVRSPXXkZQMrJYSUUTKe3eVGfj1LEV0UXtSB4GXA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771959046; c=relaxed/simple; bh=8dGV0+M7FjvoVPF6oN3ccwtkqvl3km0cRitGORMh8k0=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=t8kobo71iTtExG1bfQGZd9+vXm2B1X069vFMZO5oVoCnodj76IO527NkAMfJ9Mtssbh9tbFzThVgKrhZe3AjUoVYm2h5OXwYR6vrdJcBepKlSfdIn9WbzycJW0jsnFgWXVybX7RIPWP7skZ2cvD4sFJhBzz1X9UURH8YqzAD/3M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=paragon-software.com; spf=pass smtp.mailfrom=paragon-software.com; dkim=pass (1024-bit key) header.d=paragon-software.com header.i=@paragon-software.com header.b=ZE7WyS8L; arc=none smtp.client-ip=35.157.23.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=paragon-software.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=paragon-software.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=paragon-software.com header.i=@paragon-software.com header.b="ZE7WyS8L" Received: from relayfre-01.paragon-software.com (unknown [176.12.100.13]) by relayaws-01.paragon-software.com (Postfix) with ESMTPS id 8DF6C1D13; Tue, 24 Feb 2026 18:48:54 +0000 (UTC) Authentication-Results: relayaws-01.paragon-software.com; dkim=pass (1024-bit key; unprotected) header.d=paragon-software.com header.i=@paragon-software.com header.b=ZE7WyS8L; dkim-atps=neutral Received: from dlg2.mail.paragon-software.com (vdlg-exch-02.paragon-software.com [172.30.1.105]) by relayfre-01.paragon-software.com (Postfix) with ESMTPS id 79C2221FA; Tue, 24 Feb 2026 18:50:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paragon-software.com; s=mail; t=1771959042; bh=63Irec/xWzAFWxR2O6FavOSUajfciH+rIsneyv7bR+o=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=ZE7WyS8LHUUk3DzL18eieNGBZitWGSzo1jxVlcB9l0GI2hsLWBMTXmeHWK3maje2G 3tBktFOU2IJilh64InfqFwEcbD9FJiRkzf3llg1IhMwx29p0aQROG1a1PV5tlf6mDn J1Mx48DCvDEYxigsvrkpLmwXGo98Su+3k191TIIE= Received: from [192.168.95.128] (172.30.20.212) by vdlg-exch-02.paragon-software.com (172.30.1.105) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.7; Tue, 24 Feb 2026 21:50:41 +0300 Message-ID: <19dd94a6-e290-4575-9624-eeee3c64df57@paragon-software.com> Date: Tue, 24 Feb 2026 19:50:38 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] fs/ntfs3: resolve compare function in public index APIs To: Adarsh Das CC: , References: <20260208083904.63963-1-adarshdas950@gmail.com> Content-Language: en-US From: Konstantin Komarov In-Reply-To: <20260208083904.63963-1-adarshdas950@gmail.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: vdlg-exch-02.paragon-software.com (172.30.1.105) To vdlg-exch-02.paragon-software.com (172.30.1.105) On 2/8/26 09:39, Adarsh Das wrote: > Previously the comparator was stored in struct ntfs_index and > used by low-level helpers such as hdr_find_e(). This creates > a dependency on index state in private helpers. > > Resolve the compare function in the public index APIs and pass > it explicitly to internal helpers. > > This should make the ownership of the comparator explicit and keeps > low-level index code independent of index-root policy. > > This also resolves the TODO comment about dropping the stored > comparator from struct ntfs_index. > > Signed-off-by: Adarsh Das > --- > fs/ntfs3/index.c | 76 ++++++++++++++++++++++++++++++---------------- > fs/ntfs3/ntfs_fs.h | 5 +-- > 2 files changed, 50 insertions(+), 31 deletions(-) > > diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c > index 7157cfd70fdc..f04a8e13f8b8 100644 > --- a/fs/ntfs3/index.c > +++ b/fs/ntfs3/index.c > @@ -716,10 +716,10 @@ static bool fnd_is_empty(struct ntfs_fnd *fnd) > */ > static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx, > const struct INDEX_HDR *hdr, const void *key, > - size_t key_len, const void *ctx, int *diff) > + size_t key_len, const void *ctx, int *diff, > + NTFS_CMP_FUNC cmp) > { > struct NTFS_DE *e, *found = NULL; > - NTFS_CMP_FUNC cmp = indx->cmp; > int min_idx = 0, mid_idx, max_idx = 0; > int diff2; > int table_size = 8; > @@ -729,9 +729,6 @@ static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx, > u32 total = le32_to_cpu(hdr->total); > u16 offs[128]; > > - if (unlikely(!cmp)) > - return NULL; > - > fill_table: > if (end > total) > return NULL; > @@ -802,7 +799,8 @@ static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx, > static struct NTFS_DE *hdr_insert_de(const struct ntfs_index *indx, > struct INDEX_HDR *hdr, > const struct NTFS_DE *de, > - struct NTFS_DE *before, const void *ctx) > + struct NTFS_DE *before, const void *ctx, > + NTFS_CMP_FUNC cmp) > { > int diff; > size_t off = PtrOffset(hdr, before); > @@ -825,7 +823,7 @@ static struct NTFS_DE *hdr_insert_de(const struct ntfs_index *indx, > } > /* No insert point is applied. Get it manually. */ > before = hdr_find_e(indx, hdr, de + 1, le16_to_cpu(de->key_size), ctx, > - &diff); > + &diff, cmp); > if (!before) > return NULL; > off = PtrOffset(hdr, before); > @@ -917,10 +915,6 @@ int indx_init(struct ntfs_index *indx, struct ntfs_sb_info *sbi, > > init_rwsem(&indx->run_lock); > > - indx->cmp = get_cmp_func(root); > - if (!indx->cmp) > - goto out; > - > return 0; > > out: > @@ -1142,6 +1136,7 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni, > int err; > struct NTFS_DE *e; > struct indx_node *node; > + NTFS_CMP_FUNC cmp; > > if (!root) > root = indx_get_root(&ni->dir, ni, NULL, NULL); > @@ -1151,10 +1146,16 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni, > return -EINVAL; > } > > + cmp = get_cmp_func(root); > + if (unlikely(!cmp)) { > + WARN_ON_ONCE(1); > + return -EINVAL; > + } > + > /* Check cache. */ > e = fnd->level ? fnd->de[fnd->level - 1] : fnd->root_de; > if (e && !de_is_last(e) && > - !(*indx->cmp)(key, key_len, e + 1, le16_to_cpu(e->key_size), ctx)) { > + !(*cmp)(key, key_len, e + 1, le16_to_cpu(e->key_size), ctx)) { > *entry = e; > *diff = 0; > return 0; > @@ -1164,7 +1165,7 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni, > fnd_clear(fnd); > > /* Lookup entry that is <= to the search value. */ > - e = hdr_find_e(indx, &root->ihdr, key, key_len, ctx, diff); > + e = hdr_find_e(indx, &root->ihdr, key, key_len, ctx, diff, cmp); > if (!e) > return -EINVAL; > > @@ -1184,7 +1185,7 @@ int indx_find(struct ntfs_index *indx, struct ntfs_inode *ni, > > /* Lookup entry that is <= to the search value. */ > e = hdr_find_e(indx, &node->index->ihdr, key, key_len, ctx, > - diff); > + diff, cmp); > if (!e) { > put_indx_node(node); > return -EINVAL; > @@ -1583,7 +1584,7 @@ static int indx_add_allocate(struct ntfs_index *indx, struct ntfs_inode *ni, > static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > const struct NTFS_DE *new_de, > struct NTFS_DE *root_de, const void *ctx, > - struct ntfs_fnd *fnd, bool undo) > + struct ntfs_fnd *fnd, bool undo, NTFS_CMP_FUNC cmp) > { > int err = 0; > struct NTFS_DE *e, *e0, *re; > @@ -1624,7 +1625,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > if ((undo || asize + ds_root < sbi->max_bytes_per_attr) && > mi_resize_attr(mi, attr, ds_root)) { > hdr->total = cpu_to_le32(hdr_total + ds_root); > - e = hdr_insert_de(indx, hdr, new_de, root_de, ctx); > + e = hdr_insert_de(indx, hdr, new_de, root_de, ctx, cmp); > WARN_ON(!e); > fnd_clear(fnd); > fnd->root_de = e; > @@ -1765,7 +1766,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > * Now root is a parent for new index buffer. > * Insert NewEntry a new buffer. > */ > - e = hdr_insert_de(indx, hdr, new_de, NULL, ctx); > + e = hdr_insert_de(indx, hdr, new_de, NULL, ctx, cmp); > if (!e) { > err = -EINVAL; > goto out_put_n; > @@ -1795,7 +1796,7 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni, > static int > indx_insert_into_buffer(struct ntfs_index *indx, struct ntfs_inode *ni, > struct INDEX_ROOT *root, const struct NTFS_DE *new_de, > - const void *ctx, int level, struct ntfs_fnd *fnd) > + const void *ctx, int level, struct ntfs_fnd *fnd, NTFS_CMP_FUNC cmp) > { > int err; > const struct NTFS_DE *sp; > @@ -1812,7 +1813,7 @@ indx_insert_into_buffer(struct ntfs_index *indx, struct ntfs_inode *ni, > > /* Try the most easy case. */ > e = fnd->level - 1 == level ? fnd->de[level] : NULL; > - e = hdr_insert_de(indx, hdr1, new_de, e, ctx); > + e = hdr_insert_de(indx, hdr1, new_de, e, ctx, cmp); > fnd->de[level] = e; > if (e) { > /* Just write updated index into disk. */ > @@ -1889,12 +1890,12 @@ indx_insert_into_buffer(struct ntfs_index *indx, struct ntfs_inode *ni, > * (depending on sp <=> new_de). > */ > hdr_insert_de(indx, > - (*indx->cmp)(new_de + 1, le16_to_cpu(new_de->key_size), > + (*cmp)(new_de + 1, le16_to_cpu(new_de->key_size), > up_e + 1, le16_to_cpu(up_e->key_size), > ctx) < 0 ? > hdr2 : > hdr1, > - new_de, NULL, ctx); > + new_de, NULL, ctx, cmp); > > indx_mark_used(indx, ni, new_vbn >> indx->idx2vbn_bits); > > @@ -1909,14 +1910,14 @@ indx_insert_into_buffer(struct ntfs_index *indx, struct ntfs_inode *ni, > */ > if (!level) { > /* Insert in root. */ > - err = indx_insert_into_root(indx, ni, up_e, NULL, ctx, fnd, 0); > + err = indx_insert_into_root(indx, ni, up_e, NULL, ctx, fnd, 0, cmp); > } else { > /* > * The target buffer's parent is another index buffer. > * TODO: Remove recursion. > */ > err = indx_insert_into_buffer(indx, ni, root, up_e, ctx, > - level - 1, fnd); > + level - 1, fnd, cmp); > } > > if (err) { > @@ -1950,6 +1951,7 @@ int indx_insert_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > struct NTFS_DE *e; > struct ntfs_fnd *fnd_a = NULL; > struct INDEX_ROOT *root; > + NTFS_CMP_FUNC cmp; > > if (!fnd) { > fnd_a = fnd_get(); > @@ -1966,6 +1968,12 @@ int indx_insert_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > goto out; > } > > + cmp = get_cmp_func(root); > + if (unlikely(!cmp)) { > + WARN_ON_ONCE(1); > + return -EINVAL; > + } > + > if (fnd_is_empty(fnd)) { > /* > * Find the spot the tree where we want to > @@ -1989,13 +1997,13 @@ int indx_insert_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > * new entry into it. > */ > err = indx_insert_into_root(indx, ni, new_de, fnd->root_de, ctx, > - fnd, undo); > + fnd, undo, cmp); > } else { > /* > * Found a leaf buffer, so we'll insert the new entry into it. > */ > err = indx_insert_into_buffer(indx, ni, root, new_de, ctx, > - fnd->level - 1, fnd); > + fnd->level - 1, fnd, cmp); > } > > out: > @@ -2288,6 +2296,7 @@ int indx_delete_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > u32 e_size, root_size, new_root_size; > size_t trim_bit; > const struct INDEX_NAMES *in; > + NTFS_CMP_FUNC cmp; > > fnd = fnd_get(); > if (!fnd) { > @@ -2307,6 +2316,12 @@ int indx_delete_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > goto out; > } > > + cmp = get_cmp_func(root); > + if (unlikely(!cmp)) { > + WARN_ON_ONCE(1); > + return -EINVAL; > + } > + > /* Locate the entry to remove. */ > err = indx_find(indx, ni, root, key, key_len, ctx, &diff, &e, fnd); > if (err) > @@ -2372,9 +2387,9 @@ int indx_delete_entry(struct ntfs_index *indx, struct ntfs_inode *ni, > err = level ? indx_insert_into_buffer(indx, ni, root, > re, ctx, > fnd->level - 1, > - fnd) : > + fnd, cmp) : > indx_insert_into_root(indx, ni, re, e, > - ctx, fnd, 0); > + ctx, fnd, 0, cmp); > kfree(re); > > if (err) > @@ -2668,6 +2683,7 @@ int indx_update_dup(struct ntfs_inode *ni, struct ntfs_sb_info *sbi, > struct INDEX_ROOT *root; > struct mft_inode *mi; > struct ntfs_index *indx = &ni->dir; > + NTFS_CMP_FUNC cmp; > > fnd = fnd_get(); > if (!fnd) > @@ -2679,6 +2695,12 @@ int indx_update_dup(struct ntfs_inode *ni, struct ntfs_sb_info *sbi, > goto out; > } > > + cmp = get_cmp_func(root); > + if (unlikely(!cmp)) { > + WARN_ON_ONCE(1); > + return -EINVAL; > + } > + > /* Find entry in directory. */ > err = indx_find(indx, ni, root, fname, fname_full_size(fname), sbi, > &diff, &e, fnd); > diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h > index a4559c9f64e6..63d3b76e7d06 100644 > --- a/fs/ntfs3/ntfs_fs.h > +++ b/fs/ntfs3/ntfs_fs.h > @@ -192,9 +192,6 @@ struct ntfs_index { > /* read/write access to 'bitmap_run'/'alloc_run' while ntfs_readdir */ > struct rw_semaphore run_lock; > > - /*TODO: Remove 'cmp'. */ > - NTFS_CMP_FUNC cmp; > - > u8 index_bits; // log2(root->index_block_size) > u8 idx2vbn_bits; // log2(root->index_block_clst) > u8 vbn2vbo_bits; // index_block_size < cluster? 9 : cluster_bits > @@ -378,7 +375,7 @@ struct ntfs_inode { > */ > u8 mi_loaded; > > - /* > + /* > * Use this field to avoid any write(s). > * If inode is bad during initialization - use make_bad_inode > * If inode is bad during operations - use this field Hello, Queued for the next merge window, thanks. Regards, Konstantin