Protect pending_status in UploadPending (#1992)

This requires avoiding double-locking in RowFlush.  References #1991.
This commit is contained in:
Andrew Gaul 2022-07-22 23:30:04 +09:00 committed by GitHub
parent 22f2392fca
commit e0655008b3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 23 deletions

View File

@ -1384,7 +1384,7 @@ off_t FdEntity::BytesModified()
// Files smaller than the minimum part size will not be multipart uploaded,
// but will be uploaded as single part(normally).
//
int FdEntity::RowFlush(int fd, const char* tpath, bool force_sync)
int FdEntity::RowFlush(int fd, const char* tpath, AutoLock::Type type, bool force_sync)
{
S3FS_PRN_INFO3("[tpath=%s][path=%s][pseudo_fd=%d][physical_fd=%d]", SAFESTRPTR(tpath), path.c_str(), fd, physical_fd);
@ -1392,7 +1392,7 @@ int FdEntity::RowFlush(int fd, const char* tpath, bool force_sync)
return -EBADF;
}
AutoLock auto_lock(&fdent_lock);
AutoLock auto_lock(&fdent_lock, type);
// check pseudo fd and its flag
fdinfo_map_t::iterator miter = pseudo_fd_map.find(fd);
@ -1594,7 +1594,7 @@ int FdEntity::RowFlushMultipart(PseudoFdInfo* pseudo_obj, const char* tpath)
S3FS_PRN_ERR("failed to truncate file(physical_fd=%d) to zero, but continue...", physical_fd);
}
// put pending headers or create new file
if(0 != (result = UploadPending(-1))){
if(0 != (result = UploadPending(-1, AutoLock::ALREADY_LOCKED))){
return result;
}
}
@ -1722,7 +1722,7 @@ int FdEntity::RowFlushMixMultipart(PseudoFdInfo* pseudo_obj, const char* tpath)
S3FS_PRN_ERR("failed to truncate file(physical_fd=%d) to zero, but continue...", physical_fd);
}
// put pending headers or create new file
if(0 != (result = UploadPending(-1))){
if(0 != (result = UploadPending(-1, AutoLock::ALREADY_LOCKED))){
return result;
}
}
@ -1901,7 +1901,7 @@ int FdEntity::RowFlushStreamMultipart(PseudoFdInfo* pseudo_obj, const char* tpat
pseudo_obj->ClearUploadInfo(); // clear multipart upload info
// put pending headers or create new file
if(0 != (result = UploadPending(-1))){
if(0 != (result = UploadPending(-1, AutoLock::ALREADY_LOCKED))){
return result;
}
}
@ -2384,8 +2384,9 @@ bool FdEntity::MergeOrgMeta(headers_t& updatemeta)
// global function in s3fs.cpp
int put_headers(const char* path, headers_t& meta, bool is_copy, bool use_st_size = true);
int FdEntity::UploadPending(int fd)
int FdEntity::UploadPending(int fd, AutoLock::Type type)
{
AutoLock auto_lock(&fdent_lock, type);
int result;
if(NO_UPDATE_PENDING == pending_status){
@ -2410,7 +2411,7 @@ int FdEntity::UploadPending(int fd)
S3FS_PRN_ERR("could not create a new file(%s), because fd is not specified.", path.c_str());
result = -EBADF;
}else{
result = Flush(fd, true);
result = Flush(fd, AutoLock::ALREADY_LOCKED, true);
if(0 != result){
S3FS_PRN_ERR("failed to flush for file(%s) by(%d).", path.c_str(), result);
}else{

View File

@ -114,7 +114,7 @@ class FdEntity
int GetPhysicalFd() const { return physical_fd; }
bool IsModified() const;
bool MergeOrgMeta(headers_t& updatemeta);
int UploadPending(int fd);
int UploadPending(int fd, AutoLock::Type type);
bool GetStats(struct stat& st, bool lock_already_held = false);
int SetCtime(struct timespec time, bool lock_already_held = false);
@ -137,8 +137,8 @@ class FdEntity
int Load(off_t start, off_t size, AutoLock::Type type, bool is_modified_flag = false); // size=0 means loading to end
off_t BytesModified();
int RowFlush(int fd, const char* tpath, bool force_sync = false);
int Flush(int fd, bool force_sync = false) { return RowFlush(fd, NULL, force_sync); }
int RowFlush(int fd, const char* tpath, AutoLock::Type type, bool force_sync = false);
int Flush(int fd, AutoLock::Type type, bool force_sync = false) { return RowFlush(fd, NULL, type, force_sync); }
ssize_t Read(int fd, char* bytes, off_t start, size_t size, bool force_load = false);
ssize_t Write(int fd, const char* bytes, off_t start, size_t size);

View File

@ -1155,7 +1155,7 @@ static int s3fs_symlink(const char* _from, const char* _to)
}
}
// upload
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_WARN("could not upload tmpfile(result=%d)", result);
}
}
@ -1287,7 +1287,7 @@ static int rename_object_nocopy(const char* from, const char* to, bool update_ct
}
// upload
if(0 != (result = ent->RowFlush(autoent.GetPseudoFd(), to, true))){
if(0 != (result = ent->RowFlush(autoent.GetPseudoFd(), to, AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", to, result);
return result;
}
@ -1526,7 +1526,7 @@ static int s3fs_rename(const char* _from, const char* _to)
AutoFdEntity autoent;
FdEntity* ent;
if(NULL != (ent = autoent.OpenExistFdEntity(from, O_RDWR))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", to, result);
return result;
}
@ -1719,7 +1719,7 @@ static int s3fs_chmod_nocopy(const char* _path, mode_t mode)
ent->SetMode(mode);
// upload
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", strpath.c_str(), result);
return result;
}
@ -1904,7 +1904,7 @@ static int s3fs_chown_nocopy(const char* _path, uid_t uid, gid_t gid)
ent->SetGId(gid);
// upload
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", strpath.c_str(), result);
return result;
}
@ -2136,7 +2136,7 @@ static int s3fs_utimens_nocopy(const char* _path, const struct timespec ts[2])
}
// upload
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", strpath.c_str(), result);
return result;
}
@ -2210,7 +2210,7 @@ static int s3fs_truncate(const char* _path, off_t size)
// The cause is unknown now, but it can be avoided by flushing the file.
//
if(0 == size){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", path, result);
return result;
}
@ -2236,7 +2236,7 @@ static int s3fs_truncate(const char* _path, off_t size)
S3FS_PRN_ERR("could not open file(%s): errno=%d", path, errno);
return -EIO;
}
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), true))){
if(0 != (result = ent->Flush(autoent.GetPseudoFd(), AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", path, result);
return result;
}
@ -2311,7 +2311,7 @@ static int s3fs_open(const char* _path, struct fuse_file_info* fi)
time_t now = time(NULL);
struct timespec ts = {now, 0};
ent->SetMCtime(ts, ts);
if(0 != (result = ent->RowFlush(autoent.GetPseudoFd(), path, true))){
if(0 != (result = ent->RowFlush(autoent.GetPseudoFd(), path, AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", path, result);
StatCache::getStatCacheData()->DelStat(path);
return result;
@ -2372,7 +2372,7 @@ static int s3fs_write(const char* _path, const char* buf, size_t size, off_t off
if(max_dirty_data != -1 && ent->BytesModified() >= max_dirty_data){
int flushres;
if(0 != (flushres = ent->RowFlush(static_cast<int>(fi->fh), path, true))){
if(0 != (flushres = ent->RowFlush(static_cast<int>(fi->fh), path, AutoLock::NONE, true))){
S3FS_PRN_ERR("could not upload file(%s): result=%d", path, flushres);
StatCache::getStatCacheData()->DelStat(path);
return flushres;
@ -2430,7 +2430,7 @@ static int s3fs_flush(const char* _path, struct fuse_file_info* fi)
if(NULL != (ent = autoent.GetExistFdEntity(path, static_cast<int>(fi->fh)))){
ent->UpdateMtime(true); // clear the flag not to update mtime.
ent->UpdateCtime();
result = ent->Flush(static_cast<int>(fi->fh), false);
result = ent->Flush(static_cast<int>(fi->fh), AutoLock::NONE, false);
StatCache::getStatCacheData()->DelStat(path);
}
S3FS_MALLOCTRIM(0);
@ -2455,7 +2455,7 @@ static int s3fs_fsync(const char* _path, int datasync, struct fuse_file_info* fi
ent->UpdateMtime();
ent->UpdateCtime();
}
result = ent->Flush(static_cast<int>(fi->fh), false);
result = ent->Flush(static_cast<int>(fi->fh), AutoLock::NONE, false);
}
S3FS_MALLOCTRIM(0);
@ -2498,7 +2498,8 @@ static int s3fs_release(const char* _path, struct fuse_file_info* fi)
return -EIO;
}
int result = ent->UploadPending(static_cast<int>(fi->fh));
// TODO: correct locks held?
int result = ent->UploadPending(static_cast<int>(fi->fh), AutoLock::NONE);
if(0 != result){
S3FS_PRN_ERR("could not upload pending data(meta, etc) for pseudo_fd(%llu) / path(%s)", (unsigned long long)(fi->fh), path);
return result;