Skip to content

Commit abe0050

Browse files
guptaskguptask
committed
Moved common segment code for posix and file shm segments into ShmCommon
1 parent 33deea7 commit abe0050

14 files changed

+299
-426
lines changed

‎cachelib/allocator/CacheAllocator-inl.h‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
7373
config_.accessConfig.getNumBuckets()),
7474
nullptr,
7575
ShmSegmentOpts(config_.accessConfig.getPageSize(),
76-
false, config_.usePosixShm))
76+
false,
77+
config_.usePosixShm))
7778
.addr,
7879
compressor_,
7980
[this](Item* it) -> WriteHandle { return acquire(it); })),
@@ -85,7 +86,8 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
8586
config_.chainedItemAccessConfig.getNumBuckets()),
8687
nullptr,
8788
ShmSegmentOpts(config_.accessConfig.getPageSize(),
88-
false, config_.usePosixShm))
89+
false,
90+
config_.usePosixShm))
8991
.addr,
9092
compressor_,
9193
[this](Item* it) -> WriteHandle { return acquire(it); })),
@@ -102,7 +104,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
102104
config_.isNvmCacheTruncateAllocSizeEnabled()} {
103105
initCommon(false);
104106
shmManager_->removeShm(detail::kShmInfoName,
105-
PosixSysVSegmentOpts(config_.usePosixShm));
107+
PosixSysVSegmentOpts(config_.usePosixShm));
106108
}
107109

108110
template <typename CacheTrait>
@@ -120,15 +122,19 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemAttachT, Config config)
120122
accessContainer_(std::make_unique<AccessContainer>(
121123
deserializer_->deserialize<AccessSerializationType>(),
122124
config_.accessConfig,
123-
shmManager_->attachShm(detail::kShmHashTableName, nullptr,
124-
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)),
125+
shmManager_->attachShm(
126+
detail::kShmHashTableName,
127+
nullptr,
128+
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)),
125129
compressor_,
126130
[this](Item* it) -> WriteHandle { return acquire(it); })),
127131
chainedItemAccessContainer_(std::make_unique<AccessContainer>(
128132
deserializer_->deserialize<AccessSerializationType>(),
129133
config_.chainedItemAccessConfig,
130-
shmManager_->attachShm(detail::kShmChainedItemHashTableName, nullptr,
131-
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)),
134+
shmManager_->attachShm(
135+
detail::kShmChainedItemHashTableName,
136+
nullptr,
137+
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm)),
132138
compressor_,
133139
[this](Item* it) -> WriteHandle { return acquire(it); })),
134140
chainedItemLocks_(config_.chainedItemsLockPower,
@@ -150,7 +156,7 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemAttachT, Config config)
150156
// this info shm segment here and the new info shm segment's size is larger
151157
// than this one, creating new one will fail.
152158
shmManager_->removeShm(detail::kShmInfoName,
153-
PosixSysVSegmentOpts(config_.usePosixShm));
159+
PosixSysVSegmentOpts(config_.usePosixShm));
154160
}
155161

156162
template <typename CacheTrait>
@@ -291,8 +297,9 @@ void CacheAllocator<CacheTrait>::initWorkers() {
291297

292298
template <typename CacheTrait>
293299
std::unique_ptr<Deserializer> CacheAllocator<CacheTrait>::createDeserializer() {
294-
auto infoAddr = shmManager_->attachShm(detail::kShmInfoName, nullptr,
295-
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm));
300+
auto infoAddr = shmManager_->attachShm(
301+
detail::kShmInfoName, nullptr,
302+
ShmSegmentOpts(PageSizeT::NORMAL, false, config_.usePosixShm));
296303
return std::make_unique<Deserializer>(
297304
reinterpret_cast<uint8_t*>(infoAddr.addr),
298305
reinterpret_cast<uint8_t*>(infoAddr.addr) + infoAddr.size);
@@ -3157,8 +3164,10 @@ void CacheAllocator<CacheTrait>::saveRamCache() {
31573164
ShmSegmentOpts opts;
31583165
opts.typeOpts = PosixSysVSegmentOpts(config_.usePosixShm);
31593166

3160-
void* infoAddr = shmManager_->createShm(detail::kShmInfoName, ioBuf->length(),
3161-
nullptr, opts).addr;
3167+
void* infoAddr =
3168+
shmManager_
3169+
->createShm(detail::kShmInfoName, ioBuf->length(), nullptr, opts)
3170+
.addr;
31623171
Serializer serializer(reinterpret_cast<uint8_t*>(infoAddr),
31633172
reinterpret_cast<uint8_t*>(infoAddr) + ioBuf->length());
31643173
serializer.writeToBuffer(std::move(ioBuf));
@@ -3513,8 +3522,8 @@ bool CacheAllocator<CacheTrait>::stopReaper(std::chrono::seconds timeout) {
35133522
}
35143523

35153524
template <typename CacheTrait>
3516-
bool CacheAllocator<CacheTrait>::cleanupStrayShmSegments(
3517-
const std::string& cacheDir, bool posix /*TODO(SHM_FILE): const std::vector<CacheMemoryTierConfig>& config */) {
3525+
bool CacheAllocator<
3526+
CacheTrait>::cleanupStrayShmSegments(const std::string& cacheDir, bool posix /*TODO(SHM_FILE): const std::vector<CacheMemoryTierConfig>& config */) {
35183527
if (util::getStatIfExists(cacheDir, nullptr) && util::isDir(cacheDir)) {
35193528
try {
35203529
// cache dir exists. clean up only if there are no other processes
@@ -3537,7 +3546,8 @@ bool CacheAllocator<CacheTrait>::cleanupStrayShmSegments(
35373546
// TODO(SHM_FILE): try to nuke segments of differente types (which require
35383547
// extra info)
35393548
// for (auto &tier : config) {
3540-
// ShmManager::removeByName(cacheDir, tierShmName, config_.memoryTiers[i].opts);
3549+
// ShmManager::removeByName(cacheDir, tierShmName,
3550+
// config_.memoryTiers[i].opts);
35413551
// }
35423552
}
35433553
return true;

‎cachelib/allocator/CacheAllocator.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,8 +1197,9 @@ class CacheAllocator : public CacheBase {
11971197
// returns true if there was no error in trying to cleanup the segment
11981198
// because another process was attached. False if the user tried to clean up
11991199
// and the cache was actually attached.
1200-
static bool cleanupStrayShmSegments(const std::string& cacheDir, bool posix
1201-
/*TODO: const std::vector<CacheMemoryTierConfig>& config = {} */);
1200+
static bool cleanupStrayShmSegments(
1201+
const std::string& cacheDir, bool posix
1202+
/*TODO: const std::vector<CacheMemoryTierConfig>& config = {} */);
12021203

12031204
// gives a relative offset to a pointer within the cache.
12041205
uint64_t getItemPtrAsOffset(const void* ptr);

‎cachelib/allocator/TempShmMapping.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ TempShmMapping::~TempShmMapping() {
3535
try {
3636
if (addr_) {
3737
shmManager_->removeShm(detail::kTempShmCacheName.str(),
38-
PosixSysVSegmentOpts(false /* posix */));
38+
PosixSysVSegmentOpts(false /* posix */));
3939
}
4040
if (shmManager_) {
4141
shmManager_.reset();
@@ -79,7 +79,7 @@ void* TempShmMapping::createShmMapping(ShmManager& shmManager,
7979
} catch (...) {
8080
if (shmAddr) {
8181
shmManager.removeShm(detail::kTempShmCacheName.str(),
82-
PosixSysVSegmentOpts(false /* posix */));
82+
PosixSysVSegmentOpts(false /* posix */));
8383
} else {
8484
munmap(addr, size);
8585
}

‎cachelib/shm/FileShmSegment.cpp‎

Lines changed: 15 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -27,165 +27,20 @@
2727
namespace facebook {
2828
namespace cachelib {
2929

30-
constexpr static mode_t kRWMode = 0666;
31-
typedef struct stat stat_t;
32-
33-
namespace detail {
34-
35-
// TODO(SHM_FILE): move those *Impl functions to common file, there are copied
36-
// from PosixShmSegment.cpp
37-
static int openImpl(const char* name, int flags) {
38-
const int fd = open(name, flags);
39-
40-
if (fd != -1) {
41-
return fd;
42-
}
43-
44-
switch (errno) {
45-
case EEXIST:
46-
case EMFILE:
47-
case ENFILE:
48-
case EACCES:
49-
util::throwSystemError(errno);
50-
break;
51-
case ENAMETOOLONG:
52-
case EINVAL:
53-
util::throwSystemError(errno, "Invalid segment name");
54-
break;
55-
case ENOENT:
56-
if (!(flags & O_CREAT)) {
57-
util::throwSystemError(errno);
58-
} else {
59-
XDCHECK(false);
60-
// FIXME: posix says that ENOENT is thrown only when O_CREAT
61-
// is not set. However, it seems to be set even when O_CREAT
62-
// was set and the parent of path name does not exist.
63-
util::throwSystemError(errno, "Invalid errno");
64-
}
65-
break;
66-
default:
67-
XDCHECK(false);
68-
util::throwSystemError(errno, "Invalid errno");
69-
}
70-
return kInvalidFD;
71-
}
72-
73-
static void unlinkImpl(const char* const name) {
74-
const int ret = unlink(name);
75-
if (ret == 0) {
76-
return;
77-
}
78-
79-
switch (errno) {
80-
case ENOENT:
81-
case EACCES:
82-
util::throwSystemError(errno);
83-
break;
84-
case ENAMETOOLONG:
85-
case EINVAL:
86-
util::throwSystemError(errno, "Invalid segment name");
87-
break;
88-
default:
89-
XDCHECK(false);
90-
util::throwSystemError(errno, "Invalid errno");
91-
}
92-
}
93-
94-
static void ftruncateImpl(int fd, size_t size) {
95-
const int ret = ftruncate(fd, size);
96-
if (ret == 0) {
97-
return;
98-
}
99-
switch (errno) {
100-
case EBADF:
101-
case EINVAL:
102-
util::throwSystemError(errno);
103-
break;
104-
default:
105-
XDCHECK(false);
106-
util::throwSystemError(errno, "Invalid errno");
107-
}
108-
}
109-
110-
static void fstatImpl(int fd, stat_t* buf) {
111-
const int ret = fstat(fd, buf);
112-
if (ret == 0) {
113-
return;
114-
}
115-
switch (errno) {
116-
case EBADF:
117-
case ENOMEM:
118-
case EOVERFLOW:
119-
util::throwSystemError(errno);
120-
break;
121-
default:
122-
XDCHECK(false);
123-
util::throwSystemError(errno, "Invalid errno");
124-
}
125-
}
126-
127-
static void* mmapImpl(
128-
void* addr, size_t length, int prot, int flags, int fd, off_t offset) {
129-
void* ret = mmap(addr, length, prot, flags, fd, offset);
130-
if (ret != MAP_FAILED) {
131-
return ret;
132-
}
133-
134-
switch (errno) {
135-
case EACCES:
136-
case EAGAIN:
137-
if (flags & MAP_LOCKED) {
138-
util::throwSystemError(ENOMEM);
139-
break;
140-
}
141-
case EBADF:
142-
case EINVAL:
143-
case ENFILE:
144-
case ENODEV:
145-
case ENOMEM:
146-
case EPERM:
147-
case ETXTBSY:
148-
case EOVERFLOW:
149-
util::throwSystemError(errno);
150-
break;
151-
default:
152-
XDCHECK(false);
153-
util::throwSystemError(errno, "Invalid errno");
154-
}
155-
return nullptr;
156-
}
157-
158-
static void munmapImpl(void* addr, size_t length) {
159-
const int ret = munmap(addr, length);
160-
161-
if (ret == 0) {
162-
return;
163-
} else if (errno == EINVAL) {
164-
util::throwSystemError(errno);
165-
} else {
166-
XDCHECK(false);
167-
util::throwSystemError(EINVAL, "Invalid errno");
168-
}
169-
}
170-
171-
} // namespace detail
172-
17330
FileShmSegment::FileShmSegment(ShmAttachT,
174-
const std::string& name,
175-
ShmSegmentOpts opts)
176-
: ShmBase(std::move(opts), name),
177-
fd_(getExisting(getPath(), opts_)) {
31+
const std::string& name,
32+
ShmSegmentOpts opts)
33+
: ShmBase(std::move(opts), name), fd_(getExisting(getPath(), opts_)) {
17834
XDCHECK_NE(fd_, kInvalidFD);
17935
markActive();
18036
createReferenceMapping();
18137
}
18238

18339
FileShmSegment::FileShmSegment(ShmNewT,
184-
const std::string& name,
185-
size_t size,
186-
ShmSegmentOpts opts)
187-
: ShmBase(std::move(opts), name),
188-
fd_(createNewSegment(getPath())) {
40+
const std::string& name,
41+
size_t size,
42+
ShmSegmentOpts opts)
43+
: ShmBase(std::move(opts), name), fd_(createNewSegment(getPath())) {
18944
markActive();
19045
resize(size);
19146
XDCHECK(isActive());
@@ -217,13 +72,15 @@ FileShmSegment::~FileShmSegment() {
21772

21873
int FileShmSegment::createNewSegment(const std::string& name) {
21974
constexpr static int createFlags = O_RDWR | O_CREAT | O_EXCL;
220-
return detail::openImpl(name.c_str(), createFlags);
75+
detail::open_func_t open_func = std::bind(open, name.c_str(), createFlags);
76+
return detail::openImpl(open_func, createFlags);
22177
}
22278

22379
int FileShmSegment::getExisting(const std::string& name,
224-
const ShmSegmentOpts& opts) {
80+
const ShmSegmentOpts& opts) {
22581
int flags = opts.readOnly ? O_RDONLY : O_RDWR;
226-
return detail::openImpl(name.c_str(), flags);
82+
detail::open_func_t open_func = std::bind(open, name.c_str(), flags);
83+
return detail::openImpl(open_func, flags);
22784
}
22885

22986
void FileShmSegment::markForRemoval() {
@@ -240,7 +97,8 @@ void FileShmSegment::markForRemoval() {
24097

24198
bool FileShmSegment::removeByPath(const std::string& path) {
24299
try {
243-
detail::unlinkImpl(path.c_str());
100+
detail::unlink_func_t unlink_func = std::bind(unlink, path.c_str());
101+
detail::unlinkImpl(unlink_func);
244102
return true;
245103
} catch (const std::system_error& e) {
246104
// unlink is opaque unlike sys-V api where its through the shmid. Hence
@@ -263,7 +121,7 @@ size_t FileShmSegment::getSize() const {
263121
return buf.st_size;
264122
} else {
265123
throw std::runtime_error(folly::sformat(
266-
"Trying to get size of segment with name {} in an invalid state",
124+
"Trying to get size of segment with name {} in an invalid state",
267125
getName()));
268126
}
269127
return 0;

‎cachelib/shm/FileShmSegment.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,16 @@ class FileShmSegment : public ShmBase {
4747
//
4848
// @param name Name of the segment
4949
// @param opts the options for attaching to the segment.
50-
FileShmSegment(ShmAttachT,
51-
const std::string& name,
52-
ShmSegmentOpts opts = {});
50+
FileShmSegment(ShmAttachT, const std::string& name, ShmSegmentOpts opts = {});
5351

5452
// create a new segment
5553
// @param name The name of the segment
5654
// @param size The size of the segment. This will be rounded up to the
5755
// nearest page size.
5856
FileShmSegment(ShmNewT,
59-
const std::string& name,
60-
size_t size,
61-
ShmSegmentOpts opts = {});
57+
const std::string& name,
58+
size_t size,
59+
ShmSegmentOpts opts = {});
6260

6361
// destructor
6462
~FileShmSegment() override;

0 commit comments

Comments
 (0)