why btt_map_read() function in the kernel not return the trim/error flag normally.

11 views
Skip to first unread message

Wu, Dennis

unread,
May 18, 2022, 5:11:08 AM5/18/22
to pmem

As the describe in the https://docs.kernel.org/driver-api/nvdimm/btt.html?highlight=btt

== ==  ====================================================

31 30  Description

== ==  ====================================================

0  0   Initial state. Reads return zeroes; Premap = Postmap

0  1   Zero state: Reads return zeroes

1  0   Error state: Reads fail; Writes clear 'E' bit

1  1   Normal Block – has valid postmap

== ==  ====================================================

 

But the function:

static int btt_map_read(struct arena_info *arena, u32 lba, u32 *mapping,  int *trim, int *error, unsigned long rwb_flags).

If the block is valid, the trim=0 and error=0, if the code check the state, it will think this is a initial state.

 

From the current logic, no issue, but later update it will easily to trigger some issues.  Can someone help to explain this implementation?

 

BR,
Dennis Wu

Wu, Dennis

unread,
May 18, 2022, 9:07:42 PM5/18/22
to pmem

In the current kernel btt.c logic, if the btt_map is in the initial state, then z_flag and e_flag are zero and I am not sure if the logic is correct? please help take a look.

 

ret = btt_map_read(arena, premap, &postmap, &t_flag, &e_flag,

                                     NVDIMM_IO_ATOMIC);

                   if (ret)

                            goto out_lane;

 

                   /*

                   * We loop to make sure that the post map LBA didn't change

                   * from under us between writing the RTT and doing the actual

                   * read.

                   */

                   while (1) {

                            u32 new_map;

                            int new_t, new_e;

 

                            if (t_flag) {

                                     zero_fill_data(page, off, cur_len);

                                     goto out_lane;

                            }

 

                            if (e_flag) {

                                     ret = -EIO;

                                     goto out_lane;

                            }

 

 

BR,
Dennis Wu

--
You received this message because you are subscribed to the Google Groups "pmem" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pmem+uns...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/pmem/SN6PR11MB2640FCF99451C73C6BC69477F8D19%40SN6PR11MB2640.namprd11.prod.outlook.com.

Vishal Verma

unread,
May 19, 2022, 12:57:44 PM5/19/22
to Wu, Dennis, pmem
Hi Dennis,

See the switch (ze) section in both btt_map_read and btt_map_write functions.
It translates the spec definitions of z/e flags to a logical API - so the function arguments set neither z/e if they want none of the flags set, but the functions internally interpret that to mean both flags need to be set.

Thanks,
Vishal

Wu, Dennis

unread,
May 19, 2022, 8:22:31 PM5/19/22
to Vishal Verma, pmem

Thank you Vishal! About the btt_map_write, I see if the e_t_flags are 00, they will be set as normal. But in the read, when the e_t_flags are “11”, they will return the 00, that means in the read, initial state and normal state can’t be differentiated.

Then in the btt_read_pg logic,  even if the btt_map are not initialized, it will read the data. The logic is a different with the user libpmemblk.

         if ((*bttp->ns_cbp->nsread)(bttp->ns, lane, &entry,

                                     sizeof(entry), map_entry_off) < 0)

                   return -1;

 

         entry = le32toh(entry);

         /*

         * Retries come back to the top of this loop (for a rare case where

         * the map is changed by another thread doing writes to the same LBA).

         */

         while (1) {

                   if (map_entry_is_error(entry)) {

                            ERR("EIO due to map entry error flag");

                            errno = EIO;

                            return -1;

                   }

 

                   if (map_entry_is_zero_or_initial(entry))

                            return zero_block(bttp, buf);

 

Br,
Dennis Wu

Wu, Dennis

unread,
May 27, 2022, 12:15:04 AM5/27/22
to Wu, Dennis, Vishal Verma, pmem

Hi, Vishal,

Any feedback on this logic?

 

Thanks a lot and best regards,
Dennis Wu

Reply all
Reply to author
Forward
0 new messages