Skip to content

Commit

Permalink
bugfix: a new written tupitem was not visible to other concurrent GPU…
Browse files Browse the repository at this point in the history
… cores, then it caused NULL-tupitem assertion.

issue reported at heterodb#658
GpuPreAgg(nogroup) write out a result tupitem by the first one.
Once the leader thread gets the role of __insertOneTupleNoGroups(),
1. it acquires the exclusive lock (kds_final->nitems == UINT_MAX)
2. it writes out an empty result tuple.
3. it updated the ROW_INDEX(kds_final) by atomic operation.
4. then, unlock the buffer (kds_final->nitems <-- 1)

However, reader side didn't use volatile read to fetch the resule
tuple-item from ROW_INDEX(kds_final), so it looks here is an empty
tuple on the row_index[0] due to code optimization.

So, we added volatile qualifier to fetch the row_index[0] not to
oversight new values written by the concurrent cores.
  • Loading branch information
kaigai committed Nov 25, 2023
1 parent 5a73ce9 commit 0d45b42
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 25 deletions.
1 change: 1 addition & 0 deletions src/cuda_gpupreagg.cu
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ __insertOneTupleNoGroups(kern_context *kcxt,
kexp_groupby_actions);
tupitem->t_len = tupsz;
tupitem->rowid = 0;
__threadfence();
__atomic_write_uint32(KDS_GET_ROWINDEX(kds_final),
__kds_packed((char *)kds_final
+ kds_final->length
Expand Down
26 changes: 1 addition & 25 deletions src/xpu_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@ KDS_GET_ROWINDEX(const kern_data_store *kds)
INLINE_FUNCTION(kern_tupitem *)
KDS_GET_TUPITEM(kern_data_store *kds, uint32_t kds_index)
{
uint32_t offset = KDS_GET_ROWINDEX(kds)[kds_index];
uint32_t offset = __volatileRead(KDS_GET_ROWINDEX(kds) + kds_index);

if (!offset)
return NULL;
Expand All @@ -1145,30 +1145,6 @@ KDS_GET_TUPITEM(kern_data_store *kds, uint32_t kds_index)
- __kds_unpack(offset));
}

/* kern_tupitem by tuple-offset */
INLINE_FUNCTION(HeapTupleHeaderData *)
KDS_FETCH_TUPITEM(kern_data_store *kds,
uint32_t tuple_offset,
ItemPointerData *p_self,
uint32_t *p_len)
{
kern_tupitem *tupitem;

Assert(kds->format == KDS_FORMAT_ROW ||
kds->format == KDS_FORMAT_HASH);
if (tuple_offset == 0)
return NULL;
Assert(tuple_offset < kds->length);
tupitem = (kern_tupitem *)((char *)kds
+ kds->length
- __kds_unpack(tuple_offset));
if (p_self)
*p_self = tupitem->htup.t_ctid;
if (p_len)
*p_len = tupitem->t_len;
return &tupitem->htup;
}

INLINE_FUNCTION(uint32_t *)
KDS_GET_HASHSLOT_BASE(const kern_data_store *kds)
{
Expand Down

0 comments on commit 0d45b42

Please sign in to comment.